andywer / webpack-blocks

📦 Configure webpack using functional feature blocks.
MIT License
2.97k stars 94 forks source link

Remove css-loader and style-loader from the postcss block #293

Closed marcofugaro closed 5 years ago

marcofugaro commented 6 years ago

Hey, this solves an issue I had with the postcss block, but it changes the way the block is used. So feel free to start a discussion, or tell me if you find a better solution to my problem.

Basically I wanted to use postcss in the production build with the mini-css-extract-plugin. To do this I need to disable the style-loader. I can do that easily in the css block using styleLoader: false. But I couldn't find a way to do it with the postcss block.

It seemed to me a cleaner solution would be to remove the style-loader and css-loader rather than adding a styleLoader option to the postcss plugin.

Let me know if you have anything against this or if I'm missing the reason the style-loader and css-loader were there in the first place.

jvanbruegge commented 6 years ago

I thought it was that way all along :sweat_smile:

marcofugaro commented 6 years ago

Glad you guys like it! Do you think we should treat this way also the sass block?

andywer commented 6 years ago

@marcofugaro Excellent point! We probably should to keep it consistent, principle of least surprises.

marcofugaro commented 6 years ago

@andywer the sass loader is different, the files have a different extension. This next basic example will not work

module.exports = createConfig([
  css(),
  sass(/* node-sass options */)
])

You have to use sass and the css block inside a match block, otherwise they will target different files.

What do you think? Wanted to check with you. Should the basic sass block usage become


const { createConfig } = require('@webpack-blocks/webpack')
const { css } = require('@webpack-blocks/assets')
const sass = require('@webpack-blocks/sass')

module.exports = createConfig([
  match('*.{sass,scss}' [
    css(),
    sass(/* node-sass options */)
  ])
])```
andywer commented 6 years ago

We could make sass() print a warning if not used within a match() block (just check context.match).

andreychev commented 6 years ago

That's awesome!

andywer commented 6 years ago

Ahh, we haven‘t merged this PR yet... Any objections why not to apply similar changes to the SASS loader (see comments above)?

vlad-zhukov commented 6 years ago

@andywer, no objections from me 👍

On a sidenote I've got a feeling the match way of using loader blocks should become the only way eventually.

andywer commented 6 years ago

I've got a feeling the match way of using loader blocks should become the only way eventually.

100% agreed! 👍 „Explicit is better than implicit“, they said 🤓