resolver: allow @-prefixed subpaths under wildcard exports#30188
Conversation
ESModule.Package.parse scanned the entire specifier for '@' to split off a package version. For wildcard 'exports' maps the matched substring can contain '@' (e.g. 'ember-source/@ember/renderer/...', 'pkg/@scope/sub') — those '@' chars aren't version delimiters, they're subpath content. The resolver misparsed them as 'pkg@version' and failed to find the module. Restrict the '@' search to specifier[offset..package.name.len]. parseName already bounded the name to the first '/' (or 2nd '/' for scoped names), so a version delimiter can only appear before that cutoff. Fixes #30187.
|
Updated 10:12 AM PT - May 3rd, 2026
❌ @robobun, your commit b76b1bf has 5 failures in
🧪 To try this PR locally: bunx bun-pr 30188That installs a local version of the PR into your bun-30188 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRestrict detection of the ChangesPackage Specifier Parsing
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 3/5 reviews remaining, refill in 18 minutes and 43 seconds. Comment |
There was a problem hiding this comment.
LGTM — the search-range narrowing is correct and the regression guard confirms version stripping still works.
Extended reasoning...
Overview
This PR changes a single function, ESModule.Package.parse in src/resolver/package_json.zig, restricting the @-version-delimiter search from the entire specifier to just the package-name span returned by parseName. The rest of the diff is reindentation plus an early return to flatten the control flow. Four new tests in test/js/bun/resolve/resolve.test.ts cover the three previously-failing shapes (pkg/@scope/..., pkg/with@sign/..., @scope/pkg/@inner/...) and a regression guard for pkg@1.0.0/sub.
Security risks
None. This is pure bounded string slicing on an in-memory specifier. The new offset < package.name.len guard ensures the slice bounds are valid (defensive against the degenerate empty-name case). No filesystem, network, or privilege boundaries are involved.
Level of scrutiny
Module resolution is a hot, correctness-critical path, so I traced the logic for each specifier shape. The key invariant holds: a real version delimiter (pkg@ver/sub or @scope/pkg@ver/sub) always sits before the first (or second, for scoped) /, which is exactly where parseName bounds package.name — so the @ is still inside specifier[offset..package.name.len] and version stripping is preserved. Any @ past that boundary is subpath content and now correctly falls through to the no-version branch. The change strictly narrows behavior for inputs that previously failed to resolve, so the blast radius of any oversight is limited to already-broken cases.
Other factors
No CODEOWNERS cover these files. The bug hunter found nothing. The PR description includes a thorough cause analysis that matches the code, and the regression test (test-pkg@1.0.0/plain/index.js) directly exercises the path that could have regressed. The real-world motivator (ember-source's wildcard exports with @ember/* subpaths) is a concrete compatibility win.
Covers the scoped-package branch of the @-delimiter search — the '@' at index 10 in '@my/pkg@1.0.0/sub' sits inside the 16-char name span, so the version extraction still fires for genuine version specifiers.
The numbers matched a '@scope/pkg' stand-in draft, not the '@my/pkg' fixture the test uses. Keep the comment fixture-agnostic.
There was a problem hiding this comment.
LGTM — the stale comment offsets were fixed in b70912e, and bounding the @ search to the parseName span is provably safe since that span already includes any @version suffix.
Extended reasoning...
Overview
This PR changes one function, ESModule.Package.parse in src/resolver/package_json.zig, to bound the @-version-delimiter search to specifier[offset..package.name.len] instead of specifier[offset..]. The rest of the Zig diff is reindentation plus an early return package and a fall-through to the existing no-version branch. Five new test cases in test/js/bun/resolve/resolve.test.ts cover @-prefixed subpaths, mid-segment @, scoped packages, and two regression guards for the existing pkg@version / @scope/pkg@version stripping.
Security risks
None. This is pure string-slicing of a module specifier. The new offset < package.name.len guard prevents the only new failure mode (an invalid [1..0] slice). No filesystem, network, auth, or user-controlled-eval surface is touched beyond what the resolver already does.
Level of scrutiny
The resolver is a hot, critical path, so I traced the logic carefully. The key correctness argument is that parseName bounds on / (not @), so when a specifier has the form pkg@1.0.0/sub or @scope/pkg@1.0.0/sub, the returned name span already contains the @version substring. Restricting the @ search to that span therefore cannot lose any true version delimiter — it only stops matching @ characters that live in the subpath (the bug). I walked through all five test fixtures (test-pkg/@scope/..., test-pkg/with@sign/..., @my/pkg/@inner/..., test-pkg@1.0.0/..., @my/pkg@1.0.0/...) and each takes the expected branch.
Other factors
My earlier inline comment about stale worked-index numbers in a test comment was addressed in b70912e (the comment is now fixture-agnostic). No CODEOWNERS entry covers src/resolver/. The bug-hunting system found no issues on the current revision, and the regression guards explicitly pin the pre-existing @version-stripping behavior so a future change can't silently break it.
windows-2019-x64-test-bun shard hit test-http-should-emit-close-when-connection-is-aborted.ts timeout 4x in build #50620. Unrelated to resolver changes — other Windows shards and the sibling windows-2019-x64-test-bun shard all passed on the same binary.
There was a problem hiding this comment.
Logic looks correct and the stale-comment nit is addressed — deferring only because resolver/specifier-parsing changes are hot-path enough that a maintainer should sign off.
Extended reasoning...
Overview
This PR changes ESModule.Package.parse in src/resolver/package_json.zig to bound the @version delimiter search to specifier[offset..package.name.len] instead of the full specifier, so that @ characters appearing in wildcard-matched subpaths (e.g. ember-source/@ember/...) are no longer misparsed as version delimiters. The control flow is restructured from if/else to nested-if + early return + fall-through, which is semantically equivalent. Five new test cases in test/js/bun/resolve/resolve.test.ts cover the bug fix and regression-guard the existing pkg@ver/sub and @scope/pkg@ver/sub version-stripping behavior.
Security risks
None identified. The change strictly narrows a string search range using a bound (package.name.len) that is already guaranteed to be <= specifier.len since parseName returns a slice of specifier. The new offset < package.name.len guard prevents any out-of-range slicing for degenerate names.
Level of scrutiny
Medium-high. The diff is small (~15 lines, mostly re-indentation) and I traced it against all five test specifiers — each hits the intended branch. However, ESModule.Package.parse runs on every bare-specifier import in Bun, so even a narrow change here has very broad blast radius. That's the only reason I'm not approving outright.
Other factors
My one prior inline comment (stale worked-offset numbers in a test comment) was addressed in b70912e by making the comment fixture-agnostic, and the thread is resolved. The bug-hunting pass on the latest revision found nothing. Test coverage is solid, including regression guards for both unscoped and scoped @version stripping. CodeRabbit had no actionable comments.
|
Build #50624 final state: 279/288 jobs passed. The 9 failures are pre-existing CI flakes unrelated to this PR:
The resolver test file |
…sh#30188) Fixes oven-sh#30187. ## Repro A wildcard `exports` pattern `"./*": "./dist/packages/*"` failed to resolve subpaths whose matched substring starts with `@`: ```console $ bun --print 'await Bun.resolve("test-pkg/@scope/sub/index.js", process.cwd())' error: Cannot find module 'test-pkg/@scope/sub/index.js' from '/tmp/test' ``` Node resolves it fine. The file exists on disk — it's purely a specifier parsing bug. Motivating real-world case: `ember-source@6.12`'s `package.json` is `{ "exports": { "./*": "./dist/packages/*" } }` and its internal subpackages live at `dist/packages/@ember/*`, `dist/packages/@glimmer/*`, `dist/packages/@simple-dom/*`. Every one of those subpaths failed in Bun. ## Cause `ESModule.Package.parse` in `src/resolver/package_json.zig` scanned the **entire** specifier for `@` to split off `pkg@version`: ```zig const offset: usize = if (package.name.len == 0 or package.name[0] != '@') 0 else 1; if (strings.indexOfChar(specifier[offset..], '@')) |at| { ... } ``` For `test-pkg/@scope/sub/index.js` this found the `@` at index 9 (start of `@scope`) and misparsed it as a version delimiter — the name became `test-pkg/`, the "version" became `scope`, and the subpath parse read from the wrong offset. The resolver then went looking for a package called `test-pkg/` in `node_modules`, didn't find it, and errored. Same shape for any `@` past the first `/`: `pkg/with@sign/sub` failed too. ## Fix `parseName` already bounds the name to the first `/` (or the second `/` for scoped names). A version delimiter `@` can only appear **within** that span — e.g. in `test-pkg@1.0.0/sub`, the `@` is at index 8 and `package.name.len` is 14. Restrict the search to `specifier[offset..package.name.len]` and any `@` outside it is subpath content, so the no-version branch fires and `parseSubpath` gets the correct slice. ## Verification All three failing forms from the issue now resolve: ```console $ bun --print 'await Bun.resolve("test-pkg/plain/index.js", process.cwd())' .../test-pkg/dist/packages/plain/index.js $ bun --print 'await Bun.resolve("test-pkg/@scope/sub/index.js", process.cwd())' .../test-pkg/dist/packages/@scope/sub/index.js $ bun --print 'await Bun.resolve("test-pkg/with@sign/sub/index.js", process.cwd())' .../test-pkg/dist/packages/with@sign/sub/index.js ``` ember-source-shaped specifiers (`@ember/*`, `@glimmer/*`, `@simple-dom/*` via a wildcard) all resolve. New tests in `test/js/bun/resolve/resolve.test.ts`: - `@`-prefixed subpath under a plain package (`test-pkg/@scope/sub`) - Mid-segment `@` (`test-pkg/with@sign/sub`) - `@`-prefixed subpath under a scoped package (`@my/pkg/@inner/bar`) - Regression guard: `pkg@1.0.0/subpath` still strips the version --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
Fixes #30187.
Repro
A wildcard
exportspattern"./*": "./dist/packages/*"failed toresolve subpaths whose matched substring starts with
@:Node resolves it fine. The file exists on disk — it's purely a specifier
parsing bug.
Motivating real-world case:
ember-source@6.12'spackage.jsonis{ "exports": { "./*": "./dist/packages/*" } }and its internalsubpackages live at
dist/packages/@ember/*,dist/packages/@glimmer/*,dist/packages/@simple-dom/*. Every one of those subpaths failed in Bun.Cause
ESModule.Package.parseinsrc/resolver/package_json.zigscanned theentire specifier for
@to split offpkg@version:For
test-pkg/@scope/sub/index.jsthis found the@at index 9 (startof
@scope) and misparsed it as a version delimiter — the name becametest-pkg/, the "version" becamescope, and the subpath parse readfrom the wrong offset. The resolver then went looking for a package
called
test-pkg/innode_modules, didn't find it, and errored.Same shape for any
@past the first/:pkg/with@sign/subfailed too.Fix
parseNamealready bounds the name to the first/(or the second/for scoped names). A version delimiter
@can only appear withinthat span — e.g. in
test-pkg@1.0.0/sub, the@is at index 8 andpackage.name.lenis 14. Restrict the search tospecifier[offset..package.name.len]and any@outside it is subpathcontent, so the no-version branch fires and
parseSubpathgets thecorrect slice.
Verification
All three failing forms from the issue now resolve:
ember-source-shaped specifiers (
@ember/*,@glimmer/*,@simple-dom/*via a wildcard) all resolve.New tests in
test/js/bun/resolve/resolve.test.ts:@-prefixed subpath under a plain package (test-pkg/@scope/sub)@(test-pkg/with@sign/sub)@-prefixed subpath under a scoped package (@my/pkg/@inner/bar)pkg@1.0.0/subpathstill strips the version