fix: the auto option for inline module syntax
#1274
Conversation
| ], | ||
| Array [ | ||
| "button.modules.css!=!./index-loader-syntax-sass.css", | ||
| ".iuzmhXWrT3_Z-zKS-rMGM { |
There was a problem hiding this comment.
Now they are CSS moduled
Codecov Report
@@ Coverage Diff @@
## master #1274 +/- ##
=======================================
Coverage 99.46% 99.46%
=======================================
Files 11 11
Lines 752 753 +1
Branches 256 258 +2
=======================================
+ Hits 748 749 +1
Misses 4 4
Continue to review full report at Codecov.
|
| const { resourcePath } = loaderContext; | ||
| const resourcePath = | ||
| // eslint-disable-next-line no-underscore-dangle | ||
| loaderContext._module.matchResource || loaderContext.resourcePath; |
There was a problem hiding this comment.
woohoo! should this also be the logic for local ident generation?
There was a problem hiding this comment.
Yep, you are right, otherwise we will get the same class names.
After fix local ident generation should we merge it and release? All is fixed?
There was a problem hiding this comment.
i think so, seems like this covers it all. hard to know for sure until i try it out on a code base. Thanks again for helping me out here
There was a problem hiding this comment.
Yep, here interesting case, when we have:
import three from './button.module.scss!=!./base64-loader?LmZvbyB7IGNvbG9yOiByZWQ7IH0=!./simple.js';
import four from './other.module.scss!=!./base64-loader?LmZvbyB7IGNvbG9yOiByZWQ7IH0=!./simple.js';
We run the loader only once, why? Because ES modules executed only once. So I don't think here bug. But we can solve it using:
// i.e. adding `?` with different values
import three from './button.module.scss!=!./base64-loader?LmZvbyB7IGNvbG9yOiByZWQ7IH0=!./simple.js?foo=bar';
import four from './other.module.scss!=!./base64-loader?LmZvbyB7IGNvbG9yOiByZWQ7IH0=!./simple.js?foo=baz';
There was a problem hiding this comment.
ah interesting, so the query param acts like a cache buster. That solution seems fine to me
There was a problem hiding this comment.
yeah in my case it'd be easy to use a stable identifier here. This wouldn't solve the className hash generation issue tho right? I'm guessing you discovered this writing a test for that?
There was a problem hiding this comment.
Solved, please check code
There was a problem hiding this comment.
arg, i'm trying this out now and _module.matchResource is null locally. Going to debug a bit to see why will report back
There was a problem hiding this comment.
If you don't have matchResource in request, it will be nullish
There was a problem hiding this comment.
yeah this is for requests with it in it. For some reason i am seeing the module get created twice, one with the matchResource and one without, the one without is what makes it to the css-loader. It maybe has to do with ESM, still trying to figure it out
| Array [ | ||
| "other.module.scss!=!./base64-loader/index.js?[ident]!./simple.js?foo=baz", | ||
| "._1KqQu-H04lA8NLg4-NKnDf { | ||
| color: red; |
There was a problem hiding this comment.
You can see here: hash for classes are different
There was a problem hiding this comment.
But note: you should use different matchResource, otherwise hash will be same
|
LGTM |
This PR contains a:
Motivation / Use-Case
fixes #1273
Breaking Changes
No
Additional Info
/cc @jquense fixed, please look