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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: disable ProvidePlugin for javascript/esm (fixes #7032) #8250
Conversation
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.
add a test case
lib/ProvidePlugin.js
Outdated
.tap("ProvidePlugin", handler); | ||
|
||
// Disable ProvidePlugin for javascript/esm, see https://github.com/webpack/webpack/issues/7032 | ||
// normalModuleFactory.hooks.parser |
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.
remove the commented code, leave only a note why it's not enabled
For maintainers only:
|
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.
Tests missing
@aladdin-add Thanks for your update. I labeled the Pull Request so reviewers will review it again. @sokra Please review the new changes. |
@sokra I updated the test case in |
@@ -10,7 +10,8 @@ module.exports = { | |||
es2015_name: ["./harmony", "default"], | |||
es2015_alias: ["./harmony", "alias"], | |||
es2015_year: ["./harmony", "year"], | |||
"this.aaa": "./aaa" | |||
"this.aaa": "./aaa", | |||
esm: "./fff.mjs" |
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.
change this to esm: "fail"
import fff.mjs
from index.js
.
Use esm
in fff.mjs
. It should not be replaced. i. e.
export function() {
return esm;
}
Currently you are not testing your change.
Signed-off-by: weiran.zsd <weiran.zsd@alibaba-inc.com>
Thank you for your pull request! The most important CI builds succeeded, we鈥檒l review the pull request soon. |
Co-Authored-By: Aladdin-ADD <weiran.zsd@outlook.com>
Thanks |
#7032
What kind of change does this PR introduce?
bugfix
Did you add tests for your changes?
updated a test case.
Does this PR introduce a breaking change?
not sure, I guess no. 馃槃
What needs to be documented once your changes are merged?