Skip to content

Commit

Permalink
Fix resolution of assets inside node_modules
Browse files Browse the repository at this point in the history
Summary: The resolution logic for assets didn't return a `failed` resolution when it checked a non-existant folder and instead threw an error. This caused the resolver to not check alternate locations which caused issues when trying to resolve assets from packages in `node_modules`.

Reviewed By: cpojer

Differential Revision: D13941700

fbshipit-source-id: 1c33eb3f9dea0788d1410fe37d299b44b5a6f7ac
  • Loading branch information
rafeca authored and facebook-github-bot committed Feb 4, 2019
1 parent ad470e1 commit ce6824c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
15 changes: 11 additions & 4 deletions packages/metro-resolver/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,17 @@ function resolveAssetFiles(
fileNameHint: string,
platform: string | null,
): Result<AssetFileResolution, FileCandidates> {
const assetNames = resolveAsset(dirPath, fileNameHint, platform);
if (assetNames != null) {
const res = assetNames.map(assetName => path.join(dirPath, assetName));
return resolvedAs(res);
try {
const assetNames = resolveAsset(dirPath, fileNameHint, platform);

if (assetNames != null) {
const res = assetNames.map(assetName => path.join(dirPath, assetName));
return resolvedAs(res);
}
} catch (err) {
if (err.code === 'ENOENT') {
return failedFor({type: 'asset', name: fileNameHint});
}
}
return failedFor({type: 'asset', name: fileNameHint});
}
Expand Down
33 changes: 33 additions & 0 deletions packages/metro/src/DeltaBundler/__tests__/resolver-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,24 @@ describe('traverseDependencies', function() {
resolver.resolve(p('/root/index.js'), './asset2.png'),
).toThrow(UnableToResolveError);
});

it('resolves assets from packages in node_modules', async () => {
setMockFileSystem({
folder: {'index.js': ''},
node_modules: {
foo: {
'package.json': JSON.stringify({name: 'foo'}),
'asset.png': '',
},
},
});

resolver = await createResolver();

expect(
resolver.resolve(p('/root/folder/index.js'), 'foo/asset.png'),
).toBe(p('/root/node_modules/foo/asset.png'));
});
});

describe('global packages', () => {
Expand Down Expand Up @@ -1824,6 +1842,21 @@ describe('traverseDependencies', function() {
p('/root/providesFoo/index-client.js'),
);
});

it('resolves assets', async () => {
setMockFileSystem({
folder: {'index.js': ''},
providesFoo: {'asset.png': ''},
});

resolver = await createResolver({
resolver: {extraNodeModules: {foo: p('/root/providesFoo')}},
});

expect(
resolver.resolve(p('/root/folder/index.js'), 'foo/asset.png'),
).toBe(p('/root/providesFoo/asset.png'));
});
});

describe('resolveRequest config param', () => {
Expand Down

0 comments on commit ce6824c

Please sign in to comment.