Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

CSS extraction includes css-hot-loader even in production #802

Closed
edmorley opened this issue Apr 18, 2018 · 0 comments · Fixed by #809
Closed

CSS extraction includes css-hot-loader even in production #802

edmorley opened this issue Apr 18, 2018 · 0 comments · Fixed by #809
Assignees
Milestone

Comments

@edmorley
Copy link
Member

The web preset supports extraction of CSS to a separate file as of #443, via @neutrinojs/style-loader and then extract-text-webpack-plugin.

The extract-text-webpack-plugin loader/plugin doesn't support HMR hot reloading itself (unlike style-loader, which works since it's just JS) - so if one wants to use it in development, then css-hot-loader must be used alongside it.

However at the moment the web preset:

This combination is confusing, since by default css-hot-loader is only being included in production - where it will never be used.

We should either:

  1. Enable extraction in both development and production, plus in development only, also use css-hot-loader
  2. Continue to enable extraction only in production by default, and remove the use of css-hot-reloader entirely
  3. Continue to enable extraction only in production by default, and make the inclusion of css-hot-reloader conditional on production too. (This would allow people to force extraction to be enabled in development, and yet still have working hot loading)

Also relevant:

As such, I'm leaning towards option (2), with the understanding that we will likely enable extraction everywhere once mini-css-extract-plugin supports it natively.

@timkelty @eliperelman - thoughts?

Whilst we're on this topic - it seems like we could improve the way we set default options. This issue in part exists because at the moment, we set the style.extract default according to production, but yet for the hot option, we always set it to true and instead later use a .when() clause to decide whether to invoke devServer etc. But this means that @neutrinojs/style-loader is passed hot: true even in production. I was wondering too if we should move defaults one layer deeper? (ie in @neutrinojs/style-loader)?

edmorley added a commit that referenced this issue Apr 20, 2018
Since the former is deprecated for CSS extraction, and is not
compatible with webpack 4. The new plugin has a much simpler API:
https://github.com/webpack-contrib/mini-css-extract-plugin

I've also chosen to remove `css-hot-loader` - since it's unnecessary
when using `style-loader` in development. ie option (2) from #802.
(Once `mini-css-extract-plugin` supports HMR we can stop using
`style-loader` entirely.)

NB: `mini-css-extract-plugin` doesn't support having multiple plugin
instances, so now we only have one instance for both standard CSS and
module CSS. This will need testing to ensure everything still works.
edmorley added a commit that referenced this issue Apr 20, 2018
Since the former is deprecated for CSS extraction, and is not
compatible with webpack 4. The new plugin has a much simpler API:
https://github.com/webpack-contrib/mini-css-extract-plugin

I've also chosen to remove `css-hot-loader` - since it's unnecessary
when using `style-loader` in development. ie option (2) from #802.
(Once `mini-css-extract-plugin` supports HMR we can stop using
`style-loader` entirely.)

NB: `mini-css-extract-plugin` doesn't support having multiple plugin
instances, so now we only have one instance for both standard CSS and
module CSS. This will need testing to ensure everything still works.
edmorley added a commit that referenced this issue Apr 20, 2018
Since the former is deprecated for CSS extraction, and is not
compatible with webpack 4. The new plugin has a much simpler API:
https://github.com/webpack-contrib/mini-css-extract-plugin

I've also chosen to remove `css-hot-loader` - since it's unnecessary
when using `style-loader` in development. ie option (2) from #802.
(Once `mini-css-extract-plugin` supports HMR we can stop using
`style-loader` entirely.)

NB: `mini-css-extract-plugin` doesn't support having multiple plugin
instances, so now we only have one instance for both standard CSS and
module CSS. This will need testing to ensure everything still works.
@edmorley edmorley mentioned this issue Apr 20, 2018
4 tasks
edmorley added a commit that referenced this issue Apr 24, 2018
Since the former is deprecated for CSS extraction, and is not
compatible with webpack 4. The new plugin has a much simpler API:
https://github.com/webpack-contrib/mini-css-extract-plugin

I've also chosen to remove `css-hot-loader` - since it's unnecessary
when using `style-loader` in development. ie option (2) from #802.
(Once `mini-css-extract-plugin` supports HMR we can stop using
`style-loader` entirely.)

NB: `mini-css-extract-plugin` doesn't support having multiple plugin
instances, so now we only have one instance for both standard CSS and
module CSS. This will need testing to ensure everything still works.
edmorley added a commit that referenced this issue Apr 25, 2018
Notable changes:
* Major version updates of `webpack`, `webpack-dev-server` and `optimize-css-assets-webpack-plugin`
* Uses the new webpack 4 `mode` option
* Removed `@neutrinojs/chunk` in favour of the `splitChunks` feature
* Removed `@neutrinojs/babel-minify` in favour of the faster `uglify-es`
* Removed redundant `ModuleConcatenationPlugin` usage
* Removed default of `NODE_ENV` from `@neutrinojs/env`
* Replaced `extract-text-webpack-plugin` with `mini-css-extract-plugin`
* Stopped pinning `webpack-sources` to v1.0.1
* Added checks to warn about changed web preset `minify` options
* Added checks to enforce that users remove any legacy `vendor` entrypoints

For more details on the webpack changes, see:
https://github.com/webpack/webpack/releases/tag/v4.0.0

Fixes #737.
Fixes #802.
Closes #748.
Closes #768.
Closes #769.
Closes #766.
@edmorley edmorley self-assigned this Apr 25, 2018
@edmorley edmorley added this to the v9 milestone Apr 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant