-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Adds a runtime variable __webpack_require__.dp to generate a dynamic path #19238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
adds a runtime variable __webpack_require__.dp to generate a dynamic path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ? |
fixes #18895
What kind of change does this PR introduce?
It just introduces one variable
__webpack_require__.dp
which 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/