Skip to content

Commit 0fc8e45

Browse files
robhoganfacebook-github-bot
authored andcommitted
Don't zombie on preexisting Haste collisions on startup
Summary: For historical reasons, Metro currently fails "hard" (actually, tries to, but enters a zombie state) when the filesystem contains Haste collisions on startup. This is mostly inherited from Jest, which prefers to fail fast on the Haste (or Jest Mock) map being in a bad state. In Metro, it isn't useful to anyone - the server is designed to gracefully handle temporary Haste collisions - warn about them, fail to resolve ambiguous Haste names with an actionable message, and recover when the collision is fixed. This diff changes the argument to `metro-file-map` to prevent a throw which would effectively hang Metro, allowing it to treat Haste collisions detected on startup exactly the same as a Haste collision created while Metro is running. Changelog: ``` - **[Fix]**: Don't enter zombie state on startup Haste collisions ``` Reviewed By: huntie Differential Revision: D66935485 fbshipit-source-id: 39c664a12bb5be8c12851cab49dec66213daa95a
1 parent 44bab52 commit 0fc8e45

File tree

2 files changed

+100
-29
lines changed

2 files changed

+100
-29
lines changed

packages/metro/src/DeltaBundler/__tests__/resolver-test.js

+96-28
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ import type {TransformResultDependency} from '../types.flow';
1616
import type {InputConfigT} from 'metro-config/src/configTypes.flow';
1717

1818
const {getDefaultConfig, mergeConfig} = require('metro-config');
19+
const {AmbiguousModuleResolutionError} = require('metro-core');
20+
const {
21+
DuplicateHasteCandidatesError,
22+
default: {H: Haste},
23+
} = require('metro-file-map');
1924
const path = require('path');
2025
const mockPlatform = process.platform;
2126

@@ -31,7 +36,9 @@ jest
3136
endianness: () => 'LE',
3237
release: () => '',
3338
}))
34-
.mock('graceful-fs', () => require('fs'));
39+
.mock('graceful-fs', () => require('fs'))
40+
.spyOn(console, 'warn')
41+
.mockImplementation(() => {});
3542

3643
jest.setTimeout(10000);
3744

@@ -1793,9 +1800,7 @@ function dep(name: string): TransformResultDependency {
17931800
});
17941801
});
17951802

1796-
test('fatals on multiple packages with the same name', async () => {
1797-
// $FlowFixMe[cannot-write]
1798-
console.warn = jest.fn();
1803+
test('resolution throws on multiple packages with the same name', async () => {
17991804
setMockFileSystem({
18001805
'index.js': '',
18011806
aPackage: {
@@ -1807,10 +1812,9 @@ function dep(name: string): TransformResultDependency {
18071812
},
18081813
});
18091814

1810-
await expect(createResolver(config)).rejects.toThrow(
1811-
'Duplicated files or mocks. Please check the console for more info',
1812-
);
1813-
expect(console.error).toHaveBeenCalledWith(
1815+
resolver = await createResolver(config);
1816+
1817+
expect(console.warn).toHaveBeenCalledWith(
18141818
[
18151819
'metro-file-map: Haste module naming collision: aPackage',
18161820
' The following files share their name; please adjust your hasteImpl:',
@@ -1823,9 +1827,26 @@ function dep(name: string): TransformResultDependency {
18231827
'',
18241828
].join('\n'),
18251829
);
1830+
1831+
expect(() =>
1832+
resolver.resolve(p('/root/index.js'), dep('aPackage')),
1833+
).toThrowError(
1834+
new AmbiguousModuleResolutionError(
1835+
p('/root/index.js'),
1836+
new DuplicateHasteCandidatesError(
1837+
'aPackage',
1838+
Haste.GENERIC_PLATFORM,
1839+
false,
1840+
new Map([
1841+
[p('/root/aPackage/package.json'), Haste.PACKAGE],
1842+
[p('/root/anotherPackage/package.json'), Haste.PACKAGE],
1843+
]),
1844+
),
1845+
).message,
1846+
);
18261847
});
18271848

1828-
test('does not support multiple global packages for different platforms', async () => {
1849+
test('resolution throws on multiple global packages for different platforms', async () => {
18291850
setMockFileSystem({
18301851
'index.js': '',
18311852
'aPackage.android.js': {
@@ -1838,10 +1859,9 @@ function dep(name: string): TransformResultDependency {
18381859
},
18391860
});
18401861

1841-
await expect(createResolver(config)).rejects.toThrow(
1842-
'Duplicated files or mocks. Please check the console for more info',
1843-
);
1844-
expect(console.error).toHaveBeenCalledWith(
1862+
resolver = await createResolver(config, 'ios');
1863+
1864+
expect(console.warn).toHaveBeenCalledWith(
18451865
[
18461866
'metro-file-map: Haste module naming collision: aPackage',
18471867
' The following files share their name; please adjust your hasteImpl:',
@@ -1858,6 +1878,23 @@ function dep(name: string): TransformResultDependency {
18581878
'',
18591879
].join('\n'),
18601880
);
1881+
1882+
expect(() =>
1883+
resolver.resolve(p('/root/index.js'), dep('aPackage')),
1884+
).toThrowError(
1885+
new AmbiguousModuleResolutionError(
1886+
p('/root/index.js'),
1887+
new DuplicateHasteCandidatesError(
1888+
'aPackage',
1889+
Haste.GENERIC_PLATFORM,
1890+
false,
1891+
new Map([
1892+
[p('/root/aPackage.android.js/package.json'), Haste.PACKAGE],
1893+
[p('/root/aPackage.ios.js/package.json'), Haste.PACKAGE],
1894+
]),
1895+
),
1896+
).message,
1897+
);
18611898
});
18621899

18631900
test('resolves global packages before node_modules packages', async () => {
@@ -2101,17 +2138,16 @@ function dep(name: string): TransformResultDependency {
21012138
});
21022139
});
21032140

2104-
test('fatals when there are duplicated haste names', async () => {
2141+
test('resolution throws when there are duplicated haste names', async () => {
21052142
setMockFileSystem({
21062143
'index.js': '',
21072144
'hasteModule.js': '@providesModule hasteModule',
21082145
'anotherHasteModule.js': '@providesModule hasteModule',
21092146
});
21102147

2111-
await expect(createResolver(config)).rejects.toThrow(
2112-
'Duplicated files or mocks. Please check the console for more info',
2113-
);
2114-
expect(console.error).toHaveBeenCalledWith(
2148+
resolver = await createResolver(config);
2149+
2150+
expect(console.warn).toHaveBeenCalledWith(
21152151
[
21162152
'metro-file-map: Haste module naming collision: hasteModule',
21172153
' The following files share their name; please adjust your hasteImpl:',
@@ -2120,6 +2156,23 @@ function dep(name: string): TransformResultDependency {
21202156
'',
21212157
].join('\n'),
21222158
);
2159+
2160+
expect(() =>
2161+
resolver.resolve(p('/root/index.js'), dep('hasteModule')),
2162+
).toThrowError(
2163+
new AmbiguousModuleResolutionError(
2164+
p('/root/index.js'),
2165+
new DuplicateHasteCandidatesError(
2166+
'hasteModule',
2167+
Haste.GENERIC_PLATFORM,
2168+
false,
2169+
new Map([
2170+
[p('/root/anotherHasteModule.js'), Haste.MODULE],
2171+
[p('/root/hasteModule.js'), Haste.MODULE],
2172+
]),
2173+
),
2174+
).message,
2175+
);
21232176
});
21242177

21252178
test('resolves a haste module before a package in node_modules', async () => {
@@ -2143,7 +2196,7 @@ function dep(name: string): TransformResultDependency {
21432196
});
21442197
});
21452198

2146-
test('fatals when a haste module collides with a global package', async () => {
2199+
test('resolution throws when a haste module collides with a global package', async () => {
21472200
setMockFileSystem({
21482201
'index.js': '',
21492202
'hasteModule.js': '@providesModule hasteModule',
@@ -2152,10 +2205,9 @@ function dep(name: string): TransformResultDependency {
21522205
},
21532206
});
21542207

2155-
await expect(createResolver(config)).rejects.toThrow(
2156-
'Duplicated files or mocks. Please check the console for more info',
2157-
);
2158-
expect(console.error).toHaveBeenCalledWith(
2208+
resolver = await createResolver(config);
2209+
2210+
expect(console.warn).toHaveBeenCalledWith(
21592211
[
21602212
'metro-file-map: Haste module naming collision: hasteModule',
21612213
' The following files share their name; please adjust your hasteImpl:',
@@ -2164,6 +2216,23 @@ function dep(name: string): TransformResultDependency {
21642216
'',
21652217
].join('\n'),
21662218
);
2219+
2220+
expect(() =>
2221+
resolver.resolve(p('/root/index.js'), dep('hasteModule')),
2222+
).toThrowError(
2223+
new AmbiguousModuleResolutionError(
2224+
p('/root/index.js'),
2225+
new DuplicateHasteCandidatesError(
2226+
'hasteModule',
2227+
Haste.GENERIC_PLATFORM,
2228+
false,
2229+
new Map([
2230+
[p('/root/aPackage/package.json'), Haste.PACKAGE],
2231+
[p('/root/hasteModule.js'), Haste.MODULE],
2232+
]),
2233+
),
2234+
).message,
2235+
);
21672236
});
21682237

21692238
test('supports collisions between haste names and global packages if they have different platforms', async () => {
@@ -2215,17 +2284,16 @@ function dep(name: string): TransformResultDependency {
22152284
});
22162285
});
22172286

2218-
test('fatals when a filename uses a non-supported platform and there are collisions', async () => {
2287+
test('warns when a filename uses a non-supported platform and there are collisions', async () => {
22192288
setMockFileSystem({
22202289
'index.js': '',
22212290
'hasteModule.js': '@providesModule hasteModule',
22222291
'hasteModule.invalid.js': '@providesModule hasteModule',
22232292
});
22242293

2225-
await expect(createResolver(config)).rejects.toThrow(
2226-
'Duplicated files or mocks. Please check the console for more info',
2227-
);
2228-
expect(console.error).toHaveBeenCalledWith(
2294+
resolver = await createResolver(config);
2295+
2296+
expect(console.warn).toHaveBeenCalledWith(
22292297
[
22302298
'metro-file-map: Haste module naming collision: hasteModule',
22312299
' The following files share their name; please adjust your hasteImpl:',

packages/metro/src/node-haste/DependencyGraph.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@ class DependencyGraph extends EventEmitter {
101101
type: 'dep_graph_loading',
102102
hasReducedPerformance: !!hasReducedPerformance,
103103
});
104-
const fileMap = createFileMap(config, {watch});
104+
const fileMap = createFileMap(config, {
105+
throwOnModuleCollision: false,
106+
watch,
107+
});
105108

106109
// We can have a lot of graphs listening to Haste for changes.
107110
// Bump this up to silence the max listeners EventEmitter warning.

0 commit comments

Comments
 (0)