Adds a runtime variable __webpack_require__.dp to generate a dynamic path #19238
Conversation
adds a runtime variable __webpack_require__.dp to generate a dynamic path
There was a problem hiding this comment.
Copilot reviewed 6 out of 11 changed files in this pull request and generated no comments.
Files not reviewed (5)
- test/configCases/asset-url/dynamic/index.css: Language not supported
- lib/asset/AssetGenerator.js: Evaluated as low risk
- lib/RuntimeTemplate.js: Evaluated as low risk
- eslint.config.js: Evaluated as low risk
- lib/APIPlugin.js: Evaluated as low risk
Comments suppressed due to low confidence (2)
test/configCases/asset-url/dynamic/index.js:2
- The test case should include scenarios where webpack_dynamic_public_path is not defined to ensure fallback behavior is correctly handled.
__webpack_dynamic_public_path__ = (filename, publicPath) => publicPath + "hey/" + filename
lib/RuntimeGlobals.js:392
- [nitpick] The variable name dynamicPublicPath should be reviewed for consistency and clarity. Consider renaming it to something more descriptive.
module.exports.dynamicPublicPath = "__webpack_require__.dp";
There was a problem hiding this comment.
Sorry, that is a wrong solution. We need to allow developer provide own dynamic path for JS files, not just generate extra code.
Anyway we can accept this, but it requires to be __webpack_public_path__: string | function, because we already have __webpack_public_path__ and we don't need to introduce other things.
|
Hey @alexander-akait , I just have one question, what’d be the signature of the function of __webpack_public_path__ = (filename) => "hey/" + filename; |
|
@arkapratimc Yes, you are right |
| { expr: RuntimeGlobals.publicPath }, | ||
| filename | ||
| ); | ||
| assetPath = runtimeTemplate.generatePublicPath(filename); |
There was a problem hiding this comment.
We should do it every where we have RuntimeGlobals.publicPath
|
Does this work for chunk loading? |
|
@Jack-Works no, missing in this PR, anyway it is just |
|
hey @alexander-akait, test passes on one ubuntu machine https://dev.azure.com/webpack/webpack/_build/results?buildId=20771&view=logs&j=059dd913-30b4-5dab-74c6-1d726b9b84c8&t=0f11fa17-217a-5071-18f9-b54ec876c796&l=54 , but fails on another https://github.com/webpack/webpack/actions/runs/14623185311/job/41028317052?pr=19238#step:12:8 . Any idea on whats causing this ? |
You need check the |
CodSpeed Performance ReportMerging #19238 will not alter performanceComparing Summary
Benchmarks breakdown
|
|
@hai-x hi, thanks, done - https://github.com/webpack/webpack/pull/19238/files#diff-07626cfb7da83ee25692aef62aec9dc6a32d6f0511b00e6b342121c5a87b5c75R596 Tests on the same machine are still failing ... (1) https://dev.azure.com/webpack/webpack/_build/results?buildId=20859&view=logs&j=059dd913-30b4-5dab-74c6-1d726b9b84c8&t=0f11fa17-217a-5071-18f9-b54ec876c796&l=54 (2) https://github.com/webpack/webpack/actions/runs/14811503127/job/41586744979?pr=19238#step:12:8 do you think, that making a separate PR can solve this, since it was created 3 months ago ? |
fixes #18895
What kind of change does this PR introduce?
It just introduces one variable
__webpack_require__.dpwhich generates a dynamic path. Users can write__webpack_dynamic_public_path__ = (filename, publicPath) => ....to generate too.Did you add tests for your changes?
Yes
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
A note of what
__webpack_dynamic_public_path__does in https://webpack.js.org/api/module-variables/