Skip to content

docs(plugins) Add mini-css-extract-plugin #2028

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

Merged
merged 5 commits into from
Apr 16, 2018

Conversation

EugeneHlushko
Copy link
Member

@EugeneHlushko EugeneHlushko commented Apr 12, 2018

  • remove extract-text-webpack-plugin
  • add mini-css-extract-plugin
  • add redirect from extract-text-webpack-plugin to mini-css-extract-plugin
  • fix guides asset management
  • fix guides production

fixes #2016

@@ -59,16 +61,19 @@ __webpack.config.js__
+ {
+ test: /\.css$/,
+ use: [
+         'style-loader',
+         MiniCssExtractPlugin.loader,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work with hot reloading, at least not yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest we add a note that hot reloading wont work for now with MiniCssExtractPlugin or is there a known workaround? @Kovensky

Copy link
Member

@Jessidhia Jessidhia Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The known workaround is to use style-loader when hot reloading is enabled 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so we probably can skip updating this guide for now (asset-management) and leave it with style-loader?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides this, the changes look good to me if they were properly tested.

Is it necessary to add some warning box on other plugin page to let the developer know that this one should be used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a redirect from deleted plugin is added for that, also the text of the page says why it came to replace ETWP: https://github.com/webpack-contrib/mini-css-extract-plugin

Otherwise works fine locally.

@Kovensky am i right we dont need to update asset-management guide for now?

@EugeneHlushko EugeneHlushko force-pushed the feature/mini-css-extract-plugin branch from 475d01d to d119b6e Compare April 13, 2018 07:11
@EugeneHlushko
Copy link
Member Author

ok cleaned up asset-management.md, its good now from my perspective. Would really appreciate a re-review @jeremenichelli @Kovensky

@EugeneHlushko
Copy link
Member Author

EugeneHlushko commented Apr 13, 2018

Reading through #1854 we shouldnt remove redundant plugins pages but add a deprecation warning to their pages (this means pull requests into readme.md of respective plugins repos). Should i probably remove the redirect and plugin fetch removal from this PR then? CC @montogeek

@jeremenichelli
Copy link
Member

I just spotted the redirect @EugeneHlushko, yes, we don't delete or do that for deprecated plugins, we just add a deprecation warning similar to the one in here https://webpack.js.org/plugins/commons-chunk-plugin/

Consider teams that are not ready to upgrade and need this documentation for their use case. I think that with that change this would be good to go.

@EugeneHlushko
Copy link
Member Author

@jeremenichelli done 👍

Copy link
Member

@jeremenichelli jeremenichelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the comments Eugene 👍

@EugeneHlushko EugeneHlushko changed the title docs(plugins) Add mini-css-extract-plugin remove ETWP docs(plugins) Add mini-css-extract-plugin Apr 16, 2018
@montogeek montogeek merged commit dfa645c into master Apr 16, 2018
@montogeek
Copy link
Member

Thanks

@EugeneHlushko EugeneHlushko deleted the feature/mini-css-extract-plugin branch April 16, 2018 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mini-css-extract-plugin is missing from plugins
5 participants