feat: track static esm namespace keys#13972
Conversation
github-actions
Bot
added
the
release: feature
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32e1141a03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || evaluated.is_null() | ||
| || evaluated.is_undefined() | ||
| { | ||
| evaluated.as_string() |
There was a problem hiding this comment.
When a const key evaluates to a number whose Rust formatting differs from JavaScript ToPropertyKey, this stores the wrong export name. For example const key = -0; ns[key] accesses property "0" at runtime, but evaluated.as_string() records "-0", so an export such as export { zero as "0" } can be considered unused while a non-existent "-0" export is referenced; the same affects key in ns. Please use JavaScript property-key stringification for numeric values before tagging them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for tracking statically evaluable computed keys when accessing ESM namespace imports, so tree-shaking can preserve precisely referenced exports for patterns like ns[key] and key in ns.
Changes:
- Added a new tree-shaking config case covering
ns[constKey]andconstKey in ns - Extended member-chain extraction to evaluate side-effect-free computed keys only when rooted at an ESM namespace import
- Introduced parser plugin tagging/evaluation for
constprimitive property-key variables
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rspack-test/configCases/tree-shaking/esm-namespace-static-key/rspack.config.js | Adds config enabling usedExports/sideEffects behavior for the new test case |
| tests/rspack-test/configCases/tree-shaking/esm-namespace-static-key/package.json | Marks the case as side-effect free at package level |
| tests/rspack-test/configCases/tree-shaking/esm-namespace-static-key/lib.js | Adds exports plus __webpack_exports_info__ usage assertions for tree-shaking expectations |
| tests/rspack-test/configCases/tree-shaking/esm-namespace-static-key/index.js | New test covering const computed keys and in operator on ESM namespace |
| crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs | Enables safe computed-key evaluation only for ESM namespace member roots |
| crates/rspack_plugin_javascript/src/parser_plugin/mod.rs | Re-exports ESM specifier tag/data for cross-module use |
| crates/rspack_plugin_javascript/src/parser_plugin/esm_import_dependency_parser_plugin.rs | Tags side-effect-free const primitive values as “static property keys” and exposes them to evaluation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn evaluate_identifier( | ||
| &self, | ||
| parser: &mut JavascriptParser, | ||
| for_name: &str, | ||
| start: u32, | ||
| end: u32, | ||
| ) -> Option<BasicEvaluatedExpression<'static>> { | ||
| if for_name != ESM_STATIC_PROPERTY_KEY_TAG { | ||
| return None; | ||
| } | ||
| let tag_info = parser | ||
| .definitions_db | ||
| .expect_get_tag_info(parser.current_tag_info?); | ||
| let data = ESMStaticPropertyKeyData::downcast(tag_info.data.clone()?); | ||
| Some(evaluate_to_string(data.value.to_string(), start, end)) | ||
| } |
JSerFeng
force-pushed
the
fy/esm-namespace-static-key
branch
from
32e1141 to
6139173
Compare
Merging this PR will degrade performance by 3.92%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | rust@scan_dependencies@three_module |
21.2 ms | 22.1 ms | -3.92% |
Comparing JSerFeng:fy/esm-namespace-static-key (629d12d) with main (259b48a)
Footnotes
-
25 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
JSerFeng
force-pushed
the
fy/esm-namespace-static-key
branch
from
6139173 to
629d12d
Compare
JSerFeng
force-pushed
the
fy/esm-namespace-static-key
branch
from
629d12d to
a7026a4
Compare
Summary
ns[key]andkey in nspreserving precise referenced exportsRelated links
N/A
Checklist
Testing
cargo fmt --all --checkcargo check -p rspack_plugin_javascriptPATH=/Users/bytedance/.nvm/versions/node/v22.21.1/bin:$PATH pnpm --dir tests/rspack-test run test -t "esm-namespace-static-key"