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

Direct integration with mini-css-extract-plugin #14

Open
jantimon opened this issue Feb 17, 2019 · 3 comments
Open

Direct integration with mini-css-extract-plugin #14

jantimon opened this issue Feb 17, 2019 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@jantimon
Copy link

Hey @SassNinja

it took me way to long to discover this repo.
In many projects we face the problem your plugin is trying to solve.

I had an idea which would probably cause a lot of work but should simplify the configuration and performance of your plugin.

What if the mini-css-extract-plugin would introduce a hook here:

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/b653641e7993eb28fad70c1733dc45feafea93c5/src/index.js#L200

and here:

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/b653641e7993eb28fad70c1733dc45feafea93c5/src/index.js#L174

So you could split the files generated by mini-css-extract-plugin whenever they are generated or regenerated.

The configuration could look like the following:

module.exports = {
    module: {
        rules: [
            {
                test: /\.scss$/,
                use: [
                    MiniCssExtractPlugin.loader,
                    'css-loader',
                    'postcss-loader',
                    'sass-loader'
                ]
            }
        ]
    },
   plugins: [
       new MiniCssExtractPlugin(),
       new MediaQueryPlugin()
   ]
};

Maybe you could also provide some defaults e.g. split print styles and all styles for viewports larger than (e.g.) 1000px ?

This would provide an awesome zero config experience and people could just add the plugin to every project without worrying about details. (at least for basic usage)

@SassNinja
Copy link
Owner

Hi @jantimon

So you could split the files generated by mini-css-extract-plugin whenever they are generated or regenerated.

This sounds great!
Actually I've plans to bind my plugin more to the mini-css-extract-plugin and abandon support of anything else (such as style loader what doesn't make sense in production).

Would you please describe a bit more in detail what you mean with adding hooks?
(I'm neither familiar with chunkTemplates.hooks nor with mainTemplate.hooks)
Do you mean sth like this in the mini-css-extract-plugin?

if (renderedModules.length > 0) {

  // tapable hook here with possibility to modify the result
  // and adjust the chunk (by removing the extracted CSS)

  result.push({
    render: () =>
      ...

Maybe you could also provide some defaults e.g. split print styles and all styles for viewports larger than (e.g.) 1000px ?

Sounds like presets – I really like this idea, too :)
I can imagine presets for common frameworks (such as Bootstrap and Foundation for Sites) and maybe one with ranges that map certain queries automatically to e.g. mobile.

However the rework of the plugin (closer to mini-css-extract-plugin) should be done first.

@SassNinja SassNinja added the enhancement New feature or request label Feb 18, 2019
@SassNinja SassNinja added this to the 2.0.0 milestone Feb 18, 2019
@jantimon
Copy link
Author

jantimon commented Feb 18, 2019

Yes that's exactly what I had in mind about the mini-css-extract-plugin.

I just came across one issue with the media query splitting approach in general.
Take a look at the following example:

@media screen and (min-width: 992px) {
  body {
    background-color: blue;
  }
}

body {
  background-color: purple;
}

The background would always be purple as the specificities are equal but purple comes last.
If we now in production split it into two files and change the load order it would change the behaviour and suddenly the background would be blue for large devices.

Do you think that's a big issue?

@SassNinja
Copy link
Owner

Yes that's exactly what I had in mind about the mini-css-extract-plugin.

Ok, I need to check the code more in detail to know if/how this can be achieved.
Feel free to submit a PR at the mini-css-extract-plugin if already have an idea how to implement those hooks!

The background would always be purple as the specificities are equal but purple comes last.
If we now in production split it into two files and change the load order it would change the behaviour and suddenly the background would be blue for large devices.

We also discussed this in the team when talking about media query extraction and right, it may cause the side effects you described.
This may happen in particular when using frameworks and overriding some CSS later but without any media query.

Example:

// framework
h1 {
  font-size: 1rem;
}
@media screen and (min-width: 40em) {
  h1 {
    font-size: 1.5rem;
  }
}

// custom
h1 {
  font-size: 2rem;
}

Do you think that's a big issue?

I think it requires a very good structure of your CSS and knowledge about specificity. A methodology such as BEM can help you to avoid those problems.

Imo the media query extraction just reveals problems of the own structure if it causes problems and instead of abandoning it you should try to improve your code to get rid of the side effects.

Splitting is a powerful way to significantly improve the page performance – if done correctly.

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

No branches or pull requests

2 participants