-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
feature: support experimental .sass\.scss #69
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.
Thank you for tackling this.
I left some comments, could you add integration/end-to-end tests as well?
// Hack our way to disable errors on node_modules CSS modules | ||
const nextErrorCssLoader = nextCssLoaders.oneOf.find( | ||
(rule) => rule.use && rule.use.loader === 'error-loader' && regexEqual(rule.test, /\.module\.css$/) | ||
(rule) => rule.use && rule.use.loader === 'error-loader' && (Array.isArray(rule.test) ? regexEqual(rule.test[0], /\.module\.css$/) : regexEqual(rule.test, /\.module\.css$/)) |
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.
the [0]
should be avoided imo, if we can find a more dynamic way to do it, that'd be great
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.
It is either one regexp, that catches .module.css, or array of regexps, one of which captures it.
Either way function will be long if it would be written in such way. Probably that part of code should be written with ifs to be more readable.
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.
Other way is to do
rule.some((regex) => regexEqual(regex, /\.module\.css$/))
Sadly i have no idea how to do e2e/integration tests. |
Everything is already setup, you just have to add tests about the features that are supported by Next.js. Could be something like that:
Some inspiration: |
Well it depends on canary version of next. Should we wait till 9.3? vercel/next.js#10571 according to that PR sass is not experimental anymore |
You have two options here 😄
Your move ;) |
Closing in favor of #74 |
This will fix #66