Repository navigation
Conversation
Add resolver.unstable_applyBrowserSpecAfterExports. When enabled, a package's "exports" field resolves the requested subpath first, and "browser" spec redirections apply to the resolved file instead of to the requested subpath, matching webpack and Vite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Metro review: Looks goodRisk: low · Commit: Adds an opt-in resolver flag that applies "browser" redirections to the file resolved from "exports" instead of to the requested subpath, and every new code path is gated on it. The only remaining issue is a rare nit: a redirect to a directory with an invalid Findings
Verified
Not run: Two commands were refused for lack of an approval path and not retried: (1) resolving the published packages, a pnpm-style symlinked package and a real asset target through a real Automated review by Claude. Advisory only: it never approves or blocks, and a maintainer makes the call. |
| if (redirectedPath === resolution.filePath) { | ||
| return resolvedAs(resolution); | ||
| } | ||
| return resolveModulePath(context, redirectedPath, platform); |
There was a problem hiding this comment.
should fix · A "browser" redirect to a missing file after "exports" silently drops the resolution
redirectPackageExportsResolution passes the redirected path to resolveModulePath and returns its failed result as is. In resolvePackage, that failure goes up to resolveModuleFromTargetPath, which returns null, so the node_modules walk moves on to the next level.
Example: a package has "exports": {"./broken": "./broken-node.js"} and "browser": {"./broken-node.js": "./does-not-exist.js"}, and broken-node.js exists.
- Flag off:
pkg/brokenresolves tobroken-node.js. - Flag on: the resolution is thrown away with no warning. The import ends in
FailedToResolveNameError("Module does not exist in the Haste module map or in these directories: …"), which names only thenode_modulesdirectories it searched. It says nothing about the"browser"entry or the target file. If a parentnode_moduleshas another copy of the package, that copy is used silently.
The self-reference path, by contrast, throws FailedToResolvePathError with candidates. So the same broken entry fails differently depending on how the package was reached.
This breaks the resolver's "errors are data" contract. It also turns a working import into an opaque failure only because the flag was turned on. Existing "exports" handling of a missing target is the model to follow: it raises InvalidPackageConfigurationError with the paths involved, logs a warning and falls back. Suggested fix: when the redirected path fails to resolve, either throw an InvalidPackageConfigurationError that names the package, the "exports" target and the redirect target, or log a warning and keep the "exports" result. Add a test for this case.
Reproduced: Repro __review__/__tests__/browser-after-exports-review-test.js ('redirect to a missing file after exports'), run on Linux and Windows, logged: broken -> {"threw":"FailedToResolveNameError","message":"Module does not exist in the Haste module map or in these directories:\n /root/src/node_modules\n /root/node_modules\n /node_modules\n"}. Code path: redirectPackageExportsResolution (resolve.js:710) → resolvePackage returns failed → resolveModuleFromTargetPath returns null (resolve.js:431-435) → the walk continues. · id 609ce9d6
There was a problem hiding this comment.
Fixed in c9e8e89, taking your second option. A redirection to a missing file now logs an InvalidPackageConfigurationError warning that names the package, the "exports" target and the redirect target. Resolution then keeps the "exports" target, so turning on the flag can no longer break an import that resolves without it. Self-references go through the same helper, so both paths now behave the same. I added a test covering both paths; it fails on 6278f15.
A "browser" redirection of a file resolved from "exports" to a file that doesn't exist dropped the resolution, so the import failed with an opaque FailedToResolveNameError (or a self-reference with a different error). Warn and keep the "exports" target instead, so enabling the flag can't break an import that resolves without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| 1. If [`context.unstable_enablePackageExports`](#unstable_enablepackageexports-boolean) is enabled and a containing `package.json` has an `"exports"` field, get the result of [**RESOLVE_PACKAGE_EXPORTS**](#resolve_package_exports)(*context*, *packagePath*, *filePath*, *exportsField*, *platform*). | ||
| 1. If resolved path exists, return result. | ||
| 1. If resolved path exists, then | ||
| 1. If [`context.unstable_applyBrowserSpecAfterExports`](#unstable_applybrowserspecafterexports-boolean) is enabled, apply [**BROWSER_SPEC_REDIRECTION**](#browser_spec_redirection) to the resolved path. If this is `false`, return the empty module. If it is a different path, return [**RESOLVE_MODULE**](#resolve_module)(*context*, *redirectedPath*, *platform*). E.g. `"browser": {"./lib/index.node.js": "./lib/index.browser.js"}` redirects `"exports": {".": "./lib/index.node.js"}`. |
There was a problem hiding this comment.
nit · RESOLVE_PACKAGE spec step omits the missing-redirect-target fallback
Step 1.1.1 says that if the redirection is a different path, Metro returns RESOLVE_MODULE(context, redirectedPath, platform). The code does something different. If that resolution fails, redirectPackageExportsResolution logs an InvalidPackageConfigurationError warning and returns the original "exports" result (resolve.js, in redirectPackageExportsResolution). Resolution.md is meant to describe the algorithm precisely, so it should include this fallback. Suggested wording: "If that resolves, return it; otherwise log a package configuration warning and return result."
Evidence: Compared docs/Resolution.md lines 120-123 at head with redirectPackageExportsResolution in packages/metro-resolver/src/resolve.js. The test '[nonstrict] should warn and keep the "exports" target when it is redirected to a missing file' confirms the code's fallback behavior. · id 0b3fe9f6
There was a problem hiding this comment.
Fixed in 64b4ad8 using your wording: step 1.1.1 now returns RESOLVE_MODULE of the redirected path "if that resolves, otherwise log a package configuration warning and continue". Step 1.1.2 then returns the "exports" result. I also fixed the same gap in PACKAGE_SELF_RESOLVE step 5.1. It still said "return result", but self-references get the same redirection under the flag, so it now performs RESOLVE_PACKAGE step 1.1.1 first.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| if (redirectedPath === resolution.filePath) { | ||
| return resolution; | ||
| } | ||
| const result = resolveModulePath(context, redirectedPath, platform); |
There was a problem hiding this comment.
nit · A redirect to a directory with an invalid package.json throws instead of falling back
redirectPackageExportsResolution promises that enabling the flag "cannot break an import that resolves without it", and the PR description says the same. That only holds when resolveModulePath returns failed. If the redirect target is a directory whose own package.json has an unresolvable main, resolvePackageEntryPoint throws InvalidPackageError. Neither resolvePackage nor the self-reference path catches that error, so it propagates and the build fails.
Failure case: "exports": {"./dir": "./dir-node.js"}, "browser": {"./dir-node.js": "./broken-dir"}, and broken-dir/package.json with {"main": "./missing.js"}. With the flag off, pkg/dir resolves to dir-node.js. With it on, the import throws InvalidPackageError.
This package shape should be rare. Either catch InvalidPackageError around this call and use the existing warn-and-keep path, or narrow the docblock's claim to missing targets.
Reproduced: packages/metro-resolver/src/__tests__/review-2134-test.js, differential pkg/dir: flag off returns /root/node_modules/pkg/dir-node.js; flag on threw InvalidPackageError: The package /root/node_modules/pkg/broken-dir/package.json is invalid because it specifies a main module field that could not be resolved. · id 57a4d3a3
Summary
Addresses #1135 behind a new opt-in flag,
resolver.unstable_applyBrowserSpecAfterExports.For a bare import of a package that has
"exports", Metro applies "browser" spec redirections (object-valuedmainFields) to the requested subpath before it looks the subpath up in"exports". If the redirect target isn't itself an exported subpath, Metro logs "not listed in the "exports"" and falls back to file-based resolution. Two widely used packages hit this on every bundle:react-dom/server(react-dom/servercannot be resolved withunstable_enablePackageExportsand conditionbrowser#1135):"browser": {"./server.js": "./server.browser.js"}matches the./serversubpath before"exports"does.multiformats/hashes/sha2(a dependency of@atproto/*and many IPFS packages):"browser": {"./hashes/sha2": "./dist/src/hashes/sha2-browser.js"}. The maintainers would rather Metro fixed this than drop the field, which they say some webpack versions rely on (fix: resolve sha hashers via exports in react native multiformats/js-multiformats#345).The comparison in #1135 found that webpack and Vite resolve
"exports"first, then apply the object"browser"map to the resolved file. The conclusion there was that Metro should do the same. With the flag on:"exports":"exports"resolves the requested subpath, without redirecting it first. BROWSER_SPEC_REDIRECTION is then applied to the resolved file, using its closestpackage.json, includingfalse→ empty module. This coversnode_modules,nodeModulesPathsandextraNodeModuleslookups, Haste packages, and self-references."exports"target, so the flag can't break an import that resolves without it."exports": same fallback as today (a warning, then file-based resolution with redirections), except that the warning now names the requested subpath rather than the redirect target."exports", relative and absolute imports, andunstable_enablePackageExports: false.It's opt-in because results can change, not just the warnings. React Native lists
"browser"inmainFieldsbut doesn't assert thebrowsercondition on iOS/Android, so under the flagreact-dom/serveron iOS resolves toserver.node.js(the"exports"defaulttarget) instead ofserver.browser.js. With the flag off, the only new work is a boolean check. With it on, there's one extragetPackageForModulecall per file resolved from"exports", for the second pass.Not covered: webpack also applies the
"browser"map to"imports"targets. I left that out to keep this change small.Changelog: [Experimental] Add
resolver.unstable_applyBrowserSpecAfterExportsto apply "browser" field redirections to files resolved from "exports", as webpack and Vite doTest plan
New
"browser" spec redirections with "exports"block inpackage-exports-test.js. With the flag on, it covers:false→ empty modulemainFieldswithout"browser""exports"target), for both an import and a self-referenceEvery flag-on case also asserts that no warning is logged, except the two fallback cases, which each assert their one warning. One more test pins today's behavior with the flag off. Against the resolver on
main, the flag-on cases fail except themainFieldscontrol.I also ran this branch's resolver against the real published packages. Settings: Expo's defaults, i.e.
mainFields['react-native', 'browser', 'main']with conditionreact-nativefor iOS, and['browser', 'module', 'main']with conditionbrowserfor web.multiformats@14.0.5/hashes/sha2(and/sha1)sha2-browser.js+ warningsha2-browser.jssha2-browser.js+ warningsha2-browser.jsreact-dom@19.3.0/serverserver.browser.js+ warningserver.node.jsserver.browser.js+ warningserver.browser.jsmultiformats,multiformats/cid,multiformats/basics,react-domHappy to rename the flag, or to flip the default in a follow-up once it's had some use.
🤖 Generated with Claude Code