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

Prevent build errors with many small css modules #2

Merged
merged 1 commit into from
Nov 12, 2017
Merged

Prevent build errors with many small css modules #2

merged 1 commit into from
Nov 12, 2017

Conversation

terebentina
Copy link
Contributor

There's a problem when building from a codebase that is using many small css modules - you get something like

Failed to compile.
Order in extracted chunk undefined

when you run build.
A "manual" solution would be to combine all small css modules (links.module.css, buttons.module.css, etc) into a single one (say, styles.module.css) and only compose from that one. It would be a shame to, though.
ExtractTextPlugin has a fix for this namely the ignoreOrder: true option.
See https://github.com/webpack-contrib/extract-text-webpack-plugin
Discussion with the fix: webpack-contrib/extract-text-webpack-plugin#166 (comment)
Comment with the "manual" fix: css-modules/css-modules#12 (comment)

There's a problem when building from a codebase that is using many small css modules - you get something like
```
Failed to compile.
Order in extracted chunk undefined
```
when you run `build`.
A "manual" solution would be to combine all small css modules (`links.module.css`, `buttons.module.css`, etc) into a single one (say, `styles.module.css`) and only compose from that one. It would be a shame to, though.
ExtractTextPlugin has a fix for this namely the `ignoreOrder: true` option.
See https://github.com/webpack-contrib/extract-text-webpack-plugin
Discussion with the fix: webpack-contrib/extract-text-webpack-plugin#166 (comment)
Comment with the "manual" fix: css-modules/css-modules#12 (comment)
@ro-savage
Copy link
Owner

Thanks for this @terebentina and providing all the details. I will release an updated version in the next 24hrs.

We also need to add this to the cssmodules PR on facebook/create-react-app#2285.

You can also do a PR to my branch if you'd like the commit to CRA, otherwise I will add it myself.

@ro-savage ro-savage merged commit 3189e19 into ro-savage:master Nov 12, 2017
@ro-savage
Copy link
Owner

ro-savage commented Nov 13, 2017

@terebentina - I am a bit worried about how this might affect people who are not using cssmodules.

I don't understand enough about extract-text-webpack-plugin and webpack to know how important the order is for regular css. (E.g. If it means some styles will get overwritten incorrectly, or if it totally breaks some imports, etc).

@ro-savage
Copy link
Owner

ro-savage commented Nov 13, 2017

New version released as react-scripts-cssmodules@1.0.171

I've separated the cssmodules and regular css ExtractTextPlugin(), to allow for different configs while I wait for feedback.

@terebentina
Copy link
Contributor Author

Yeah, I was wondering about that too so I read more about that flag and did a few tests with just css. I didn't get any unexpected results, even with circular dependencies between the css files. Without ignoreOrder I got the message above for circular deps (with css @import) but with it I got the results I expected. I can't say I did exhaustive tests though so your approach is safer.

Anyway, it all boils down to the modules sort order: https://github.com/webpack-contrib/extract-text-webpack-plugin/blob/8de6558e33487e7606e7cd7cb2adc2cccafef272/src/lib/helpers.js#L20-L27
Normally the css files are added into the final output css in the order they are seen by the compiler (import order - so if you import from './b.css' then import from './a.css' then the contents of b.css will come before a.css in the output) but with this flag set they might be in "identifier" order (lines 24-27) so a.css before b.css

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

Successfully merging this pull request may close these issues.

Import order when composing multiple classes
2 participants