feat: explain exports/imports conditions that wrap subpaths - #676
Conversation
An exports field shaped like
"exports": {
"import": { ".": "./esm/index.js", "./*": "./esm/*.js" },
"require": "./build/bundle.js"
}
is not supported by Node.js: a condition may only map to a target or to
further conditions, so the subpaths nested under "import" are read as
condition names and match nothing. What a user saw was
"./foo" is not exported under the conditions ["import","node","default"]
from package /path (see exports field in /path/package.json)
which points at the request, not at the mistake, and reads as though the
package forgot to export "./foo". The reporter of #325 spent a day finding
the real cause.
That failure now carries the reason:
... (see exports field in /path/package.json). In that exports field,
the value at "import" is an object with subpath keys (".", "./*"),
which is not supported - a condition can only map to a target or to
further conditions, so those subpaths never match. Put the subpaths at
the top level and nest the conditions inside them instead, e.g.
{ ".": { "import": ... }, "./*": { "import": ... } }.
It is found at any depth and inside array targets, and the imports field
gets the same diagnosis for "#" keys nested under a condition.
The check is diagnostic, not validation, and that is deliberate on two
counts. Rejecting the field while building it would break packages that
resolve today - the field above still resolves "." under "require", and a
consumer who cannot fix the upstream package would have no way forward.
And it would cost a full tree walk of every package.json to catch a mistake
almost none of them make: measured over a 1000-subpath field, validating
eagerly took field construction from 0.067 to 0.289 ms, roughly 4x. Running
it only where a request has already failed puts construction back at
main's cost (0.0648 vs 0.0674 ms, retained heap identical at 2.83 KB) and
leaves resolve throughput unmoved - interleaved benchmark runs gave 9089
and 9217 ops/s against main's 8811 and 9141, a gap smaller than the spread
within either arm.
Errors from processing either field now name the package.json as well.
They carried no path at all before - it appeared only in the resolver log,
which is off unless a caller passes one.
Closes #325
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aGHrb1YaEvaGwNELGEHjF
🦋 Changeset detectedLatest commit: 22bbd7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #676 +/- ##
==========================================
+ Coverage 98.44% 98.46% +0.02%
==========================================
Files 52 52
Lines 10753 10903 +150
==========================================
+ Hits 10586 10736 +150
Misses 167 167
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughThe change adds Priority: ➖ Normal Severity of issue fixed: Medium 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
lib/util/entrypoints.jsESLint failed to execute (timeout). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov flagged one uncovered line in the new code: the condition used in the suggested example when the offending mapping is the exports field itself, rather than something nested inside it. That branch cannot run. It needs a field that mixes condition names with subpath keys, and `buildExportsField` rejects exactly that before any processor exists, so the plugin never reaches the diagnosis with one. The line was not missing a test - it was dead. Removing it collapses the shorthand special case entirely: both forms of `exports` and `imports` hold targets in their top-level values, so descending into the values and never inspecting the field's own keys is correct for all of them, and the trail can no longer be empty. Behaviour is unchanged - the issue's field is still explained under "import" and still resolves "." under "require". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aGHrb1YaEvaGwNELGEHjF
Merging this PR will improve performance by 17.45%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | node-compare: node require.resolve x 1000 |
59.3 KB | 50.5 KB | +17.55% |
| ⚡ | Simulation | realistic-midsize: mixed batch (cold cache) |
18.1 ms | 15.4 ms | +17.34% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/enhanced-resolve-660-qo14h7 (22bbd7b) with main (e1bdb75)
|
CodSpeed is red ("Performance Regression: -7.81%"). This PR does touch The flagged case is bench.add(`node-compare: node require.resolve x ${BATCH_SIZE}`, () => {
for (let i = 0; i < requests.length; i++) {
requireAnchor.resolve(requests[i]);
}
});
Two further points against reading it as real:
Independently of the benchmark: the diagnosis this PR adds runs only where So there is nothing to fix and no fix to port. I am re-running the benchmark workflow once; if it comes back red on the same warning, it is measurement noise rather than this diff. Acknowledging it in the CodSpeed dashboard is a maintainer action, so I am leaving that to you. Generated by Claude Code |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## enhanced-resolve@5.26.0 ### Minor Changes - Add experimental support for [Node.js package maps](https://nodejs.org/api/packages.html#package-maps) through a new `packageMap` option, which takes the path of the configuration file (or a `file:` `URL`) or an already-parsed `packages` object. When it is set, a bare specifier is resolved through the importing package's `dependencies` table and the target package's location is handed to the regular pipeline, instead of walking `node_modules`; relative and absolute requests and `node:` builtins are unaffected. Because several package entries may share one `url`, the package a request resolved into is exposed as `packageId` on the result and can be passed back in as `context.packageId` to resolve from that package unambiguously. Package maps are stability 1 (experimental) in Node.js, and this option tracks that specification and may change with it. (by [@alexander-akait](https://github.com/alexander-akait) in [#667](#667)) - Explain an `exports`/`imports` field whose conditions wrap subpaths, instead of failing with a message that points at the request. A field shaped like `{ "import": { ".": "./esm/index.js", "./*": "./esm/*.js" }, "require": "./build/bundle.js" }` is not supported by Node.js: the subpaths inside a condition are read as condition names, so they match nothing, and every request into the package failed as `"./foo" is not exported under the conditions [...]` — which reads as though the package forgot to export `./foo`. Such a failure now names the offending keys and shows the arrangement that works, with the subpaths at the top level and the conditions nested inside them. Resolution itself is unchanged: the diagnosis runs only on a request that has already failed, so nothing that resolves today starts failing, and a successful resolve does no extra work. Errors raised while processing either field also name the `package.json` they came from, which previously only appeared in the resolver log. (by [@alexander-akait](https://github.com/alexander-akait) in [#676](#676)) - Generate the published type declarations with TypeScript instead of `webpack/tooling`, which is no longer a dependency. Every name the package exported before is still exported, and `types.d.ts` is still the entry point, but the declarations themselves now live in `types/` and are emitted by `tsc` from the JSDoc in `lib/`. Two shapes follow the sources more closely than the previous generator did: the object form of `Plugin` no longer declares `this: Resolver` on `apply` (it is called as `plugin.apply(resolver)`, so `this` is the plugin), and the entries of `ResolveContext.stack` declare `name: string | undefined` rather than an optional `name`. Class fields that the old generator dropped, such as the cache backends on `CachedInputFileSystem`, are now part of the declarations. (by [@alexander-akait](https://github.com/alexander-akait) in [#675](#675)) ### Patch Changes - Size the ancestor path and segment arrays that `getPathsCached` keeps to what they actually hold: a `push`-built store keeps room for 17 entries while a path has a handful, and the cache holds these for the filesystem's lifetime. (by [@alexander-akait](https://github.com/alexander-akait) in [#681](#681)) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 participant
Summary
An
exportsfield shaped like{ "import": { ".": "./esm/index.js", "./*": "./esm/*.js" }, "require": "./build/bundle.js" }is not supported by Node.js: a condition may only map to a target or to further conditions, so the subpaths under"import"are read as condition names and match nothing. The failure surfaced as"./foo" is not exported under the conditions [...], which points at the request rather than the mistake and reads as though the package forgot to export./foo— #325 took its reporter a day to diagnose. That error now carries the reason: the offending keys, and the arrangement that works with the subpaths at the top level and the conditions nested inside them. Found at any depth and inside array targets, with the same diagnosis for#keys under a condition inimports. Closes #325.What kind of change does this PR introduce?
feat — a better error message; resolution behaviour is unchanged.
Did you add tests for your changes?
Yes —
test/entrypoints.test.jscovers the shorthand form, a nested offender, an offender inside an array target, theimportscounterpart, and the cases that must stay silent. The 1616 existing tests are unchanged and pass.Does this PR introduce a breaking change?
No, and deliberately so. The diagnosis runs only where a request has already failed, never while building the field, so nothing that resolves today starts failing — the field above still resolves
.underrequire. Validating the field instead would have broken that, leaving a consumer who cannot patch the upstream package with no way forward, and it would have cost a full tree walk of everypackage.jsonto catch a mistake almost none of them make: on a 1000-subpath field that took construction from 0.0674 ms to 0.2888 ms. As written, construction stays at main's cost (0.0648 vs 0.0674 ms, retained heap identical at 2.83 KB) and resolve throughput is unmoved — interleaved runs of theexportsbenchmarks gave 9089 and 9217 ops/s against main's 8811 and 9141, a gap smaller than the spread within either arm.If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a — the changeset covers the changelog.
Use of AI
Written with Claude Code. It measured rather than assumed at each step: it reproduced the current behaviour first (under
importthe field resolves nothing at all, not just subpaths), instrumented a benchmark run to confirm the field processor is built once across 3823 samples, and benchmarked an eager-validation version against this one, which is what moved the design from rejecting the field to diagnosing the failure. Reviewed by me before opening.🤖 Generated with Claude Code
https://claude.ai/code/session_016aGHrb1YaEvaGwNELGEHjF
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
exportsandimportsconfigurations by identifying problematic subpaths and showing the supported structure.package.jsonfile for easier troubleshooting.Tests
exportsandimportsconfigurations, including valid and invalid field formats.