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
feat: support css esModule generator options #18357
base: main
Are you sure you want to change the base?
Conversation
For maintainers only:
|
|
||
it("should able to require the css module as commonjs", () => { | ||
const style = require("./style.module.css"); | ||
console.log(style) |
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.
Let's remove it from here
// TODO webpack 6: remove /\[moduleid\]/.test | ||
if ( | ||
/\[id\]/.test(this.localIdentName) || | ||
/\[moduleid\]/.test(this.localIdentName) |
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.
I think we can improve it in future PRs
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.
I don't think we really need the esModule
option here, let's do it always, because we already return reasons in getConcatenationBailoutReason
Could you elaborate? Actually I didn't understand what you mean. Did you mean we don't need the |
What kind of change does this PR introduce?
feature, introduce
esModule: false
for css generator optionsDid you add tests for your changes?
Added
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
esModule css generator options