Skip to content

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

arkapratimc
Copy link
Contributor

@arkapratimc arkapratimc commented Feb 16, 2025

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/

adds a runtime variable __webpack_require__.dp to generate a dynamic path
@TheLarkInn TheLarkInn requested a review from Copilot February 18, 2025 19:45
Copy link

@Copilot Copilot AI left a 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";

Copy link
Member

@alexander-akait alexander-akait left a 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.

@arkapratimc
Copy link
Contributor Author

Hey @alexander-akait , I just have one question, what’d be the signature of the function of __webpack_public_path__ ?
For example, will it be like this →

__webpack_public_path__ = (filename) => "hey/" + filename;

@alexander-akait
Copy link
Member

@arkapratimc Yes, you are right

{ expr: RuntimeGlobals.publicPath },
filename
);
assetPath = runtimeTemplate.generatePublicPath(filename);
Copy link
Member

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

@Jack-Works
Copy link
Contributor

Does this work for chunk loading?

@alexander-akait
Copy link
Member

@Jack-Works no, missing in this PR, anyway it is just __webpack_public_path__ improvement, in the original issue we need to allow define custom public path function in build time, not runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow public path to be a function
3 participants