From cb2b8864db84a914d52244c35421f74f210d72a2 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 2 Jun 2026 04:35:25 -0700 Subject: [PATCH] Resolver: Fix browser-spec bare-specifier->relative path redirect for nested/first-party package.jsons Summary: The [browser spec](https://github.com/defunctzombie/package-browser-field-spec) redirect for bare specifiers (`"browser": {"foo-pkg": "./shims/foo.js"}`) was previously rebased against a path derived by slicing `originModulePath` after the last `node_modules/` segment. That happens to be correct only in the (common) case that the closest `package.json` is the one at the root of a containing `node_modules` package. However: - It's legal for an NPM package to contain nested `package.json` files and those may contain redirects - in which case their targets are relative to themselves, not to the package root `package.json`. - For origin modules that aren't located inside a `node_modules` tree (e.g. project-local source), `lastIndexOf('node_modules/')` returns `-1`, producing a malformed root path and either failed resolutions or surprising successes. This diff replaces that derivation with `closestPackageToOrigin.rootPath`, which we already look up at the top of `resolve`. Behaviour is identical in the already-working cases, and corrected otherwise. Matches webpack's `AliasFieldPlugin`, which rebases against `request.descriptionFileRoot`. Changelog: ``` - **[Fix]**: Fix browser-spec redirects of bare specifier to relative path, where `package.json` is not directly under `node_modules//` ```` Reviewed By: huntie Differential Revision: D105485945 --- .../src/__tests__/browser-spec-test.js | 57 +++++++++++++++++++ packages/metro-resolver/src/resolve.js | 20 +++---- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/packages/metro-resolver/src/__tests__/browser-spec-test.js b/packages/metro-resolver/src/__tests__/browser-spec-test.js index c3122002ee..9b4f326008 100644 --- a/packages/metro-resolver/src/__tests__/browser-spec-test.js +++ b/packages/metro-resolver/src/__tests__/browser-spec-test.js @@ -99,4 +99,61 @@ describe('browser field spec', () => { }); }); }); + + describe('replace specific files', () => { + test('should resolve a bare-specifier redirect relative to the origin package root, not its containing directory', () => { + // Per the browser spec, paths in the `browser` map are relative to the + // package.json file location. When the origin module lives in a + // subdirectory of its package (here `lib/nested/`), the redirect target + // must still resolve against the package root. + const packageJson = { + name: 'origin-pkg', + main: 'lib/nested/index.js', + browser: { + 'foo-pkg': './shims/foo.js', + }, + }; + const context = { + ...createResolutionContext({ + '/root/node_modules/origin-pkg/package.json': + JSON.stringify(packageJson), + '/root/node_modules/origin-pkg/lib/nested/index.js': '', + '/root/node_modules/origin-pkg/shims/foo.js': '', + }), + originModulePath: '/root/node_modules/origin-pkg/lib/nested/index.js', + mainFields: ['browser', 'main'], + }; + + expect(Resolver.resolve(context, 'foo-pkg', null)).toEqual({ + type: 'sourceFile', + filePath: '/root/node_modules/origin-pkg/shims/foo.js', + }); + }); + + test('should resolve a bare-specifier redirect for an origin outside of `node_modules`', () => { + // Project-level package.json — there is no enclosing `node_modules` + // segment, so the old heuristic of slicing after `node_modules/` would + // misbehave. The redirect must resolve against the package root. + const context = { + ...createResolutionContext({ + '/root/project/package.json': JSON.stringify({ + name: 'project', + main: 'src/index.js', + browser: { + 'foo-pkg': './shims/foo.js', + }, + }), + '/root/project/src/index.js': '', + '/root/project/shims/foo.js': '', + }), + originModulePath: '/root/project/src/index.js', + mainFields: ['browser', 'main'], + }; + + expect(Resolver.resolve(context, 'foo-pkg', null)).toEqual({ + type: 'sourceFile', + filePath: '/root/project/shims/foo.js', + }); + }); + }); }); diff --git a/packages/metro-resolver/src/resolve.js b/packages/metro-resolver/src/resolve.js index 7f1fc583f4..ab8d9cb6cd 100644 --- a/packages/metro-resolver/src/resolve.js +++ b/packages/metro-resolver/src/resolve.js @@ -135,22 +135,16 @@ export default function resolve( // If the specifier was redirected to a relative path if ( maybeRedirectedSpecifier != null && + closestPackageToOrigin != null && // Implied by maybeRedirectedSpecifier != null isRelativeImport(maybeRedirectedSpecifier) ) { - // TODO: (robhogan) This isn't right - per browser spec: "All paths for - // browser fields are relative to the package.json file location". The - // *closest* package.json is the relevant one for browser spec, regardless - // of the closest node_modules. - - // derive absolute path /.../node_modules/originModuleDir/specifier - const fromModuleParentIdx = - originModulePath.lastIndexOf('node_modules' + path.sep) + 13; - const originModuleDir = originModulePath.slice( - 0, - originModulePath.indexOf(path.sep, fromModuleParentIdx), + // Per the "browser" spec: "All paths for browser fields are relative to + // the package.json file location". `closestPackageToOrigin` is the package + // that provided the redirect, so join relative paths to its `rootPath`. + const absPath = path.resolve( + closestPackageToOrigin.rootPath, + maybeRedirectedSpecifier, ); - - const absPath = path.join(originModuleDir, maybeRedirectedSpecifier); const result = resolveModulePath(context, absPath, platform); if (result.type === 'failed') { throw new FailedToResolvePathError(result.candidates);