-
-
Notifications
You must be signed in to change notification settings - Fork 388
fix(metro-core): improve Windows metro runtime compatibility #4453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ScriptedAlchemy
wants to merge
17
commits into
main
Choose a base branch
from
cursor/module-federation-windows-runtime-606f
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+464
−31
Open
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
216bbeb
fix(metro-core): normalize windows module paths
cursoragent 4c98cc6
chore(metro-core): format helper paths
cursoragent 75fa156
ci(module-federation): add metro windows job
cursoragent 339d121
test(metro-core): normalize path expectations
cursoragent c65be96
test(module-federation): fix metro app jest transforms
cursoragent 0e4ce11
test(module-federation): set metro app babel transform
cursoragent a946207
test(module-federation): widen metro jest transforms
cursoragent 6ccce11
Merge branch 'main' into cursor/module-federation-windows-runtime-606f
ScriptedAlchemy ace3a15
Merge branch 'main' into cursor/module-federation-windows-runtime-606f
ScriptedAlchemy e501e1e
fix(metro-core): address windows runtime review comments
ScriptedAlchemy 2164568
fix(metro-core): normalize virtual entry paths on windows
ScriptedAlchemy b140982
fix: normalize metro path handling on windows
ScriptedAlchemy 163726e
refactor(metro-core): use node absolute path checks
ScriptedAlchemy c7484fe
test(metro-core): make path utils spec windows-safe
ScriptedAlchemy cb3464c
chore: add metro windows compatibility changeset
ScriptedAlchemy 2266bbb
ci(metro-core): align windows job with e2e intent
ScriptedAlchemy 6aa30c5
Merge remote-tracking branch 'origin/main' into cursor/module-federat…
ScriptedAlchemy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@module-federation/metro': patch | ||
| --- | ||
|
|
||
| fix Metro Windows compatibility by normalizing path handling and source URL generation across absolute and relative entry paths, and tighten expose key resolution to avoid incorrect extension fallback matches. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| module.exports = { | ||
| preset: 'react-native', | ||
| transformIgnorePatterns: [ | ||
| 'node_modules[\\\\/](?!.*(react-native|@react-native)[\\\\/])', | ||
| ], | ||
| transform: { | ||
| '^.+\\.(js|jsx|ts|tsx)$': ['babel-jest', {configFile: './babel.config.js'}], | ||
| }, | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| module.exports = { | ||
| preset: 'react-native', | ||
| transformIgnorePatterns: [ | ||
| 'node_modules[\\\\/](?!.*(react-native|@react-native)[\\\\/])', | ||
| ], | ||
| transform: { | ||
| '^.+\\.(js|jsx|ts|tsx)$': ['babel-jest', {configFile: './babel.config.js'}], | ||
| }, | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| module.exports = { | ||
| preset: 'react-native', | ||
| transformIgnorePatterns: [ | ||
| 'node_modules[\\\\/](?!.*(react-native|@react-native)[\\\\/])', | ||
| ], | ||
| transform: { | ||
| '^.+\\.(js|jsx|ts|tsx)$': ['babel-jest', {configFile: './babel.config.js'}], | ||
| }, | ||
| }; |
36 changes: 36 additions & 0 deletions
36
packages/metro-core/__tests__/commands/utils/path-utils.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { | ||
| normalizeOutputRelativePath, | ||
| toFileSourceUrl, | ||
| } from '../../../src/commands/utils/path-utils'; | ||
|
|
||
| describe('commands path utils', () => { | ||
| it('normalizes relative output paths to posix separators', () => { | ||
| expect(normalizeOutputRelativePath('shared\\lodash.bundle')).toBe( | ||
| 'shared/lodash.bundle', | ||
| ); | ||
| }); | ||
|
|
||
| it('builds file urls from normalized relative paths', () => { | ||
| const parsed = new URL(toFileSourceUrl('exposed\\info.bundle')); | ||
| expect(parsed.protocol).toBe('file:'); | ||
| expect(parsed.search).toBe(''); | ||
| expect(parsed.hash).toBe(''); | ||
| expect(parsed.pathname.endsWith('/exposed/info.bundle')).toBe(true); | ||
| }); | ||
|
|
||
| it('encodes reserved characters as pathname segments', () => { | ||
| const hashParsed = new URL(toFileSourceUrl('shared/#hash.bundle')); | ||
| expect(hashParsed.hash).toBe(''); | ||
| expect(hashParsed.pathname.endsWith('/shared/%23hash.bundle')).toBe(true); | ||
|
|
||
| const queryParsed = new URL(toFileSourceUrl('shared/?query.bundle')); | ||
| expect(queryParsed.search).toBe(''); | ||
| expect(queryParsed.pathname.endsWith('/shared/%3Fquery.bundle')).toBe(true); | ||
|
|
||
| const percentParsed = new URL(toFileSourceUrl('shared/%25literal.bundle')); | ||
| expect(percentParsed.pathname.endsWith('/shared/%2525literal.bundle')).toBe( | ||
| true, | ||
| ); | ||
| }); | ||
| }); |
62 changes: 62 additions & 0 deletions
62
packages/metro-core/__tests__/plugin/babel-transformer.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
| import { vol } from 'memfs'; | ||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
| import { createBabelTransformer } from '../../src/plugin/babel-transformer'; | ||
| import type { ModuleFederationConfigNormalized } from '../../src/types'; | ||
|
|
||
| function createConfig(): ModuleFederationConfigNormalized { | ||
| return { | ||
| name: 'test-app', | ||
| filename: 'remote.js', | ||
| remotes: {}, | ||
| exposes: {}, | ||
| shared: {}, | ||
| shareStrategy: 'loaded-first', | ||
| plugins: [], | ||
| }; | ||
| } | ||
|
|
||
| describe('createBabelTransformer', () => { | ||
| afterEach(() => { | ||
| vol.reset(); | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('escapes Windows paths for require()', () => { | ||
| const realReadFileSync = fs.readFileSync.bind(fs); | ||
| vi.spyOn(fs, 'readFileSync').mockImplementation(((filePath, options) => { | ||
| const targetPath = filePath.toString(); | ||
| if (vol.existsSync(targetPath)) { | ||
| return vol.readFileSync(targetPath, options as never); | ||
| } | ||
| return realReadFileSync(filePath, options as never); | ||
| }) as typeof fs.readFileSync); | ||
| vi.spyOn(fs, 'writeFileSync').mockImplementation((( | ||
| filePath, | ||
| data, | ||
| options, | ||
| ) => { | ||
| const targetPath = filePath.toString(); | ||
| vol.mkdirSync(path.dirname(targetPath), { recursive: true }); | ||
| vol.writeFileSync(targetPath, data, options as never); | ||
| }) as typeof fs.writeFileSync); | ||
|
|
||
| const tmpDirPath = path.join('/virtual', '.mf'); | ||
| vol.mkdirSync(tmpDirPath, { recursive: true }); | ||
| const windowsPath = | ||
| 'C:\\Users\\someone\\project\\node_modules\\metro-babel-transformer\\src\\index.js'; | ||
|
|
||
| const outputPath = createBabelTransformer({ | ||
| blacklistedPaths: [], | ||
| federationConfig: createConfig(), | ||
| originalBabelTransformerPath: windowsPath, | ||
| tmpDirPath, | ||
| enableInitializeCorePatching: false, | ||
| enableRuntimeRequirePatching: false, | ||
| }); | ||
|
|
||
| const output = fs.readFileSync(outputPath, 'utf-8'); | ||
| expect(output).toContain(`require(${JSON.stringify(windowsPath)})`); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { toPosixPath } from '../../src/plugin/helpers'; | ||
|
|
||
| describe('toPosixPath', () => { | ||
| it('converts backslashes to forward slashes', () => { | ||
| expect(toPosixPath('C:\\Users\\someone\\project\\src\\index.js')).toBe( | ||
| 'C:/Users/someone/project/src/index.js', | ||
| ); | ||
| }); | ||
|
|
||
| it('leaves posix paths unchanged', () => { | ||
| expect(toPosixPath('/usr/local/bin')).toBe('/usr/local/bin'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
21 changes: 21 additions & 0 deletions
21
packages/metro-core/__tests__/plugin/rewrite-request.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { createRewriteRequest } from '../../src/plugin/rewrite-request'; | ||
|
|
||
| describe('createRewriteRequest', () => { | ||
| it('normalizes manifest rewrite paths to posix separators', () => { | ||
| const rewriteRequest = createRewriteRequest({ | ||
| config: { | ||
| projectRoot: 'C:\\repo\\app', | ||
| server: {}, | ||
| } as any, | ||
| originalEntryFilename: 'index.js', | ||
| remoteEntryFilename: 'mini.bundle', | ||
| manifestPath: 'C:\\repo\\app\\node_modules\\.mf\\mf-manifest.json', | ||
| tmpDirPath: 'C:\\repo\\app\\node_modules\\.mf', | ||
| }); | ||
|
|
||
| expect(rewriteRequest('/mf-manifest.json')).toBe( | ||
| '/[metro-project]/node_modules/.mf/mf-manifest.json', | ||
| ); | ||
| }); | ||
| }); |
112 changes: 112 additions & 0 deletions
112
packages/metro-core/__tests__/plugin/serializer.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { ConfigError } from '../../src/utils/errors'; | ||
|
|
||
| vi.mock('../../src/utils/metro-compat', () => { | ||
| const baseJSBundle = vi.fn(() => ({ mocked: true })); | ||
|
|
||
| return { | ||
| CountingSet: class CountingSet<T> extends Set<T> {}, | ||
| baseJSBundle, | ||
| bundleToString: vi.fn(() => ({ code: 'serialized-output' })), | ||
| }; | ||
| }); | ||
|
|
||
| import { getModuleFederationSerializer } from '../../src/plugin/serializer'; | ||
| import { baseJSBundle } from '../../src/utils/metro-compat'; | ||
|
|
||
| function createSerializer(exposes: Record<string, string>) { | ||
| return getModuleFederationSerializer( | ||
| { | ||
| name: 'MFExampleMini', | ||
| filename: 'mini.bundle', | ||
| remotes: {}, | ||
| exposes, | ||
| shared: {}, | ||
| shareStrategy: 'loaded-first', | ||
| plugins: [], | ||
| }, | ||
| true, | ||
| ); | ||
| } | ||
|
|
||
| function createSerializerOptions(projectRoot = '/projectRoot') { | ||
| return { | ||
| runModule: false, | ||
| modulesOnly: true, | ||
| projectRoot, | ||
| } as any; | ||
| } | ||
|
|
||
| function createGraph() { | ||
| return { | ||
| dependencies: new Map(), | ||
| } as any; | ||
| } | ||
|
|
||
| describe('getModuleFederationSerializer', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('matches expose paths when the entry path contains backslashes', async () => { | ||
| const serializer = createSerializer({ './info': './src/info.tsx' }); | ||
|
|
||
| await expect( | ||
| serializer( | ||
| '/projectRoot/src\\info.tsx', | ||
| [], | ||
| createGraph(), | ||
| createSerializerOptions(), | ||
| ), | ||
| ).resolves.toBe('serialized-output'); | ||
| expect(baseJSBundle).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('matches expose paths without extension against resolved entry files', async () => { | ||
| const serializer = createSerializer({ './info': './src/info' }); | ||
|
|
||
| await expect( | ||
| serializer( | ||
| '/projectRoot/src/info.tsx', | ||
| [], | ||
| createGraph(), | ||
| createSerializerOptions(), | ||
| ), | ||
| ).resolves.toBe('serialized-output'); | ||
| expect(baseJSBundle).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('prefers exact expose path match over extensionless fallback', async () => { | ||
| const serializer = createSerializer({ | ||
| './js': './src/info.js', | ||
| './tsx': './src/info.tsx', | ||
| }); | ||
|
|
||
| await expect( | ||
| serializer( | ||
| '/projectRoot/src/info.tsx', | ||
| [], | ||
| createGraph(), | ||
| createSerializerOptions(), | ||
| ), | ||
| ).resolves.toBe('serialized-output'); | ||
| expect(baseJSBundle).toHaveBeenCalledTimes(1); | ||
|
|
||
| const preModules = vi.mocked(baseJSBundle).mock.calls[0][1] as any[]; | ||
| expect(preModules[0].output[0].data.code).toContain('["exposed/tsx"]'); | ||
| expect(preModules[1].output[0].data.code).toContain('["exposed/tsx"]'); | ||
| }); | ||
|
|
||
| it('throws a config error when no expose entry matches', async () => { | ||
| const serializer = createSerializer({ './other': './src/other.tsx' }); | ||
|
|
||
| await expect( | ||
| serializer( | ||
| '/projectRoot/src/info.tsx', | ||
| [], | ||
| createGraph(), | ||
| createSerializerOptions(), | ||
| ), | ||
| ).rejects.toBeInstanceOf(ConfigError); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.