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

Separate css extraction from build environment #1625

Merged
merged 2 commits into from
Dec 14, 2018

Conversation

tai2
Copy link
Contributor

@tai2 tai2 commented Jul 26, 2018

webpacker currently tie CSS extraction to build environment. It extracts stylesheets to files when production build or development with no dev-server.
I want to separate it from build environment because CSS extraction is merely a user preference.
Some frontend developers might desire to put stylesheets in style tag or to emit separate css files using style-loader/url even in production.
These choices are reasonable in context of recent CSSinJS practices or HTTP/2 environment.

Another problem is that webpacker checks whether command line arguments contains a keyword webpack-dev-server to switch css extraction. It prevents to use other development tools like storybook, which contains their own webpack-dev-server, with webpacker's configuration file.
I bereave this PR simplifies the condition checking logic and provides more flexible configuration.

@gauravtiwari
Copy link
Member

Thanks @tai2 Could you please rebase with master?

@tai2
Copy link
Contributor Author

tai2 commented Dec 13, 2018

@gauravtiwari rebased 🙏🏻

@gauravtiwari gauravtiwari merged commit 41d79c9 into rails:master Dec 14, 2018
@gauravtiwari
Copy link
Member

Thanks @tai2 🙏

@DougPuchalski
Copy link

This is a breaking change, why default false?

@ericdfields
Copy link

I get why it's now a config, but I did lose a bit of time wondering why my styles suddenly broke when a stray bundle install was done on a separate branch that got merged. Definitely strongly highlight this in the readme for upgraders!

@connorshea connorshea mentioned this pull request Jan 22, 2019
@zernie
Copy link
Contributor

zernie commented Jan 23, 2019

I had to spend a lot of time debugging this problem. Please consider enabling this by default on all environments.

@DougPuchalski
Copy link

Every project needs to consider how much time a change, multiplied by the number of people it affects, can be extremely costly.

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.

5 participants