perf: remove redundant clones in ESM package maps#31025
Conversation
WalkthroughModule export/import resolution now borrows ChangesModule resolution ownership optimization
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/resolver/resolver.rs`:
- Around line 3221-3227: The match on import `kind` currently routes CSS-at
imports to `import` conditions; update the match arms in both places where you
build `conditions: match kind { ... }` (the blocks that currently list
ast::ImportKind::Require | ast::ImportKind::RequireResolve =>
&self.opts.conditions.require and fallback to &self.opts.conditions.import) to
explicitly handle ast::ImportKind::At and ast::ImportKind::AtConditional and
return &self.opts.conditions.style for those cases, leaving other import kinds
to return &self.opts.conditions.import; apply the same change at the second
construction site as well so CSS `style` conditions are restored consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8051f639-bb21-442b-9b89-d648574eb9c2
📒 Files selected for processing (3)
src/resolver/package_json.rssrc/resolver/resolver.rstest/bundler/esbuild/css.test.ts
06fc4cf to
0d40b24
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/resolver/resolver.rs`:
- Around line 4837-4842: The match that selects condition sets for package
imports currently maps only Require/RequireResolve to
self.opts.conditions.require and everything else to import; update the logic in
load_package_imports() so that ast::ImportKind::At and
ast::ImportKind::AtConditional are routed to self.opts.conditions.style, keep
Require/RequireResolve -> require, and use import for the remaining kinds;
locate the match around the `conditions:` selection in resolver.rs and replace
or extend the arms to return &self.opts.conditions.style for the
At/AtConditional cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d4fa8c81-64cb-47ab-9d1c-19178cc58bf5
📒 Files selected for processing (3)
src/resolver/package_json.rssrc/resolver/resolver.rstest/bundler/esbuild/css.test.ts
|
Closing this as superseded by #30971, which already landed the resolver borrowing changes for |
What does this PR do?
Makes ESM package map resolution faster by removing clones left over from the Rust port.
The resolver was cloning the active
ConditionsMapwhen constructingESModule, then cloning the matchedEntrywhen reading from packageexportsorimports. This changes both paths to borrow from the existing resolver and package JSON data instead.Resolution behavior stays the same for package
exports, packageimports, and CSS@importpackage resolution.I timed the changed resolver paths with
hyperfine:How did you verify your code works?
cargo fmt --all --checkgit diff --checkcargo check -p bun_resolver --target x86_64-pc-windows-msvcbun bd test test/bundler/esbuild/css.test.ts test/bundler/esbuild/packagejson.test.ts test/js/bun/resolve/import-custom-condition.test.tshyperfine