fix: render numeric chunk IDs as number literals#13604
fix: render numeric chunk IDs as number literals#13604
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be547257fe
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR reduces runtime bundle size by emitting numeric chunk IDs as JavaScript number literals (matching webpack’s output) instead of always emitting them as quoted JSON strings.
Changes:
- Added
rspack_util::json_stringify_chunk_id()to emit digit-only, non-negative integer chunk IDs without quotes. - Updated multiple runtime/chunk-format code paths (CJS/ESM/JSONP/CSS/worker/SRI) to use the new chunk ID stringifier.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rspack_util/src/lib.rs | Adds json_stringify_chunk_id() + numeric-ID detection helper. |
| crates/rspack_plugin_sri/src/runtime.rs | Uses json_stringify_chunk_id() for SRI placeholder map keys. |
| crates/rspack_plugin_runtime/src/runtime_module/utils.rs | Emits chunk-id keyed maps with numeric literals when possible. |
| crates/rspack_plugin_runtime/src/module_chunk_format.rs | Emits ESM chunk id exports using the new chunk-id stringifier. |
| crates/rspack_plugin_runtime/src/common_js_chunk_format.rs | Emits exports.ids with numeric literals where possible. |
| crates/rspack_plugin_runtime/src/array_push_callback_chunk_format.rs | Emits JSONP push chunk IDs as numeric literals where possible. |
| crates/rspack_plugin_javascript/src/dependency/worker/mod.rs | Embeds worker chunk id using json_stringify_chunk_id(). |
| crates/rspack_plugin_extract_css/src/runtime.rs | Emits CSS runtime chunk-id keyed maps with numeric literals where possible. |
Comments suppressed due to low confidence (1)
crates/rspack_plugin_runtime/src/module_chunk_format.rs:151
chunk_id_json_stringis now produced byjson_stringify_chunk_id, which may return a number literal (e.g.903) rather than a JSON string. Renaming this variable to something likechunk_id_expr/chunk_id_literalwould better reflect its contents and avoid confusion at future call sites.
let base_chunk_output_name = get_chunk_output_name(chunk, compilation).await?;
let chunk_id_json_string = rspack_util::json_stringify_chunk_id(chunk.expect_id().as_str());
let mut sources = ConcatSource::default();
sources.add(RawStringSource::from(format!(
"export const __rspack_esm_id = {chunk_id_json_string};\n",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rsdoctor Bundle Diff AnalysisFound 6 projects in monorepo, 3 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 react-10kPath:
📦 Download Diff Report: react-10k Bundle Diff 📁 react-1kPath:
📦 Download Diff Report: react-1k Bundle Diff 📁 react-5kPath:
📦 Download Diff Report: react-5k Bundle Diff Generated by Rsdoctor GitHub Action |
Merging this PR will degrade performance by 3.27%
Performance Changes
Comparing |
📦 Binary Size-limit
❌ Size increased by 6.53KB from 49.39MB to 49.39MB (⬆️0.01%) |
bc8ef1f to
ba9dd59
Compare
Webpack emits numeric chunk IDs without quotes (e.g. `push([[903],`) while rspack was wrapping them in quotes (`push([["903"],`). This caused unnecessary bytes in every chunk file. Add `json_stringify_chunk_id()` that renders valid non-negative integer IDs as number literals and falls back to JSON strings for named chunks. Update all chunk ID rendering call sites across JSONP, CJS, ESM, CSS, worker, and SRI formats.
Address review feedback: - Fix __rspack_require__.e() to pass numeric chunk IDs (was "903", now 903), ensuring type consistency with runtime === comparisons - Add MAX_SAFE_INTEGER bound check to prevent precision loss - Add json_stringify_chunk_ids() helper for array rendering - Fix missed call sites in CSS plugin, MF plugin, startup chunk dependencies, and chunk prefetch startup - Add unit tests for is_numeric_id and json_stringify_chunk_id - Rename chunk_id_json_string to chunk_id_expr for clarity - Update 9 hot snapshot files and 2 watcher test assertions
ba9dd59 to
510a74f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 843161438d
ℹ️ 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".
Summary
push([[903],) while rspack wraps them in quotes (push([["903"],), causing unnecessary output bytesjson_stringify_chunk_id()helper inrspack_utilthat renders valid non-negative integer IDs as number literals and falls back to JSON strings for named chunks (e.g."main")Test plan
cargo checkpassespnpm run build:cli:dev) succeedsreact-5kbenchmark: all 15 async chunks now use numeric IDs (push([[106],) with zero quoted numeric IDs remainingUPDATE=1)