Issue 427 #824
Issue 427 #824
Conversation
Codecov Report
@@ Coverage Diff @@
## master #824 +/- ##
==========================================
+ Coverage 92.68% 92.82% +0.13%
==========================================
Files 6 6
Lines 725 725
Branches 176 176
==========================================
+ Hits 672 673 +1
+ Misses 50 49 -1
Partials 3 3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for coming back to theming 💯
Take comments with a pinch of 🧂
FYI @greg-a-smith
| { | ||
| resourceQuery: "?dark", | ||
| use: [ | ||
| Self.loader, |
There was a problem hiding this comment.
I don't see a reference to Self. Should it be like this?
| Self.loader, | |
| MiniCssExtractPlugin.loader, |
(same for next two occurrences)
There was a problem hiding this comment.
Yes, I saw the tests. But this is the README right?
| // eslint-disable-next-line no-console | ||
| console.log(`LOADING THEME - ${newTheme}`); | ||
|
|
||
| import(/* webpackChunkName: "dark" */ "./style.scss?dark").then(() => { |
There was a problem hiding this comment.
Is the chunk name comment required here, or is it optional? If it's required, would there be any way to define it in the webpack config rather than the source?
Just thinking out loud: It might be possible to use a pitching loader to expand a static ./style.scss import to one per theme (i.e. ../style.scss?dark, ./style.scss?light, ./style.scss?n...), and then use matching resource queries.
There was a problem hiding this comment.
This is for tests and examples, for production I recommend to not use comments, because you will have less names for files, so you will send less bytes for client
There was a problem hiding this comment.
Got it, thank you. Would it make sense to revise this example to use production best practices?
There was a problem hiding this comment.
Feel free to send a PR and fix it
There was a problem hiding this comment.
I can certainly fix the Self references in the README. Stand by for that.
However, I don't actually know how to replace webpackChunkName comments with something more first class (for example, something we'd see in webpack.config.js). This is one of the reasons I've been following this issue so closely.
This PR contains a:
Motivation / Use-Case
fixes #427
Breaking Changes
No
Additional Info
Two simple scenrious:
There could be many more scenarios here - you can use
var()and load only files with with variables, you can use multiple entries and more.The main idea is using
resourceQueryandimport(/* webpackChunkName: "dark" */ "./style.scss?dark")so you can run multiple timessass-loader/etc