Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ahabhgk
Copy link
Contributor

@ahabhgk ahabhgk commented Apr 26, 2024

What kind of change does this PR introduce?

feature, introduce esModule: false for css generator options

Did 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

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)


it("should able to require the css module as commonjs", () => {
const style = require("./style.module.css");
console.log(style)
Copy link
Member

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)
Copy link
Member

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

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.

I don't think we really need the esModule option here, let's do it always, because we already return reasons in getConcatenationBailoutReason

@ahabhgk
Copy link
Contributor Author

ahabhgk commented May 9, 2024

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 generator.css.esModule: false options? or there is something we can improve in the PR?

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

Successfully merging this pull request may close these issues.

None yet

3 participants