feat(javascript): add JavascriptModulesPlugin.getChunkFilenameTemplate - #15280
feat(javascript): add JavascriptModulesPlugin.getChunkFilenameTemplate#15280kakiuwang-ui wants to merge 8 commits into
Conversation
Aligns with webpack's JavascriptModulesPlugin.getChunkFilenameTemplate so plugins can resolve the filename template rspack renders a JS chunk with. Closes web-infra-dev#10012
Merging this PR will not alter performance
Comparing Footnotes
|
|
@hardfist could you approve the CI run here when you have a moment? First contribution, so the workflows need manual approval before they will start — only the Socket and CodSpeed checks have run so far (CodSpeed reports no performance change). While it is queued, the one thing worth your call is in the description: rspack has no JS-observable equivalent of |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47756a3754
ℹ️ 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".
|
@SyMind please also take a look |
Apply review suggestions from @LingyuCoder.
getPath and getPathWithInfo are documented as taking a `Filename` (website/docs/en/api/javascript-api/compilation.mdx), which includes the function form, but the TypeScript signatures only accepted a string and the value was passed straight through to the string-only N-API method. Evaluate the callback with the path data before rendering, matching webpack's Compilation.getAssetPath. getAssetPath and getAssetPathWithInfo get the same treatment for consistency.
|
Thanks for the review @LingyuCoder — both Codex findings are real, I reproduced them. Pushed 02331fa. Doc suggestions (both applied.)
Worth noting this turned out to be narrower than a new API decision: The fix evaluates the callback with the path data before handing the result to the N-API method, matching webpack's New case Callback-valued Confirmed, with
Exposing it is not quite a one-liner though: a JS filename callback is stored as
Option 1 looks closest to webpack to me, but it touches the binding's memory behaviour, so I did not want to pick unilaterally. Which direction do you prefer? I will implement it with tests once you decide. One small thing: I set |
|
Thanks for the detailed investigation. I’m currently thinking through some broader I haven’t forgotten about this PR — please give me a little more time to settle on the direction so we don’t ask you to implement something that we may need to change again. I’ll follow up here once I have a concrete recommendation. Thanks for your patience! |
…hunk-filename-template
…late 2.2.0 shipped without this API, and the recently added compilation.runtimeTemplate section does not carry an ApiMeta either.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75e2b5cf2c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3654b1014
ℹ️ 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
Adds
JavascriptModulesPlugin.getChunkFilenameTemplate(chunk, outputOptions), aligning with webpack's static method of the same name. It lets a plugin ask which filename template rspack renders a JS chunk with, instead of re-deriving that rule by hand.The resolution order mirrors rspack's own
get_js_chunk_filename_templateincrates/rspack_core/src/options/output.rs:chunk.filenameTemplate, when the chunk carries its own templateoutput.filename, when the chunk can be initialoutput.chunkFilenameotherwiseEverything this needs was already exposed to the JS side (
chunk.filenameTemplateandchunk.canBeInitial()on the napiChunk,compilation.outputOptions), so this is a pure TypeScript addition with no binding or Rust change.Difference from webpack
webpack has a fourth branch returning
output.hotUpdateChunkFilenameforHotUpdateChunkinstances. Rspack keeps hot update chunks inside the Rust HMR pipeline and never surfaces them to JavaScript, so no chunk reachable from a plugin can be a hot update chunk. There is no JS-observable equivalent ofchunk.kind() == ChunkKind::HotUpdateto branch on. This is called out in the docs and in a code comment — happy to expose the chunk kind and add the branch instead if you would rather keep the shape identical to webpack.Test
Adds a
configCases/hookscase covering all three branches from a single entry:main— initial, no template →output.filenameasync_js— async, no template →output.chunkFilenameshared-shared_js— asplitChunkscache group withfilenameset → its own template (this one is also initial, so it pins the branch ordering)Besides asserting the exact templates, the case asserts the round trip: for every chunk,
compilation.getPath(template, { chunk, contentHashType: "javascript" })must be one ofchunk.files. That way the test checks the returned template is genuinely the one rspack rendered with, rather than restating the implementation.Related links
Checklist