media-query-plugin icon indicating copy to clipboard operation
media-query-plugin copied to clipboard

Files with same name overrides (and not creating files matching mini-css-extract-plugin)

Open davidhouweling opened this issue 6 years ago • 12 comments

I've got an application which has multiple entry points... the application has this structure

src
|- home
|    |- index.scss
|    |- index.js
|- info
|    |- index.scss
|    |- index.js

However, when using the plugin, because multiple files have index as the [name], it only spits out files as index-tablet.css and index-desktop.css.

Also, it appears the naming doesn't align with mini-css-extract-plugin which uses the entry point name. I'm using fast-sass-loader instead of sass-loader but pretty much following the same setup as the readme.md. For example, if this was my webpack.config.js file (slimmed down... each js file adds the import to the scss file):

module.exports = {
  entry: {
    home: 'src/home/index.js',
    info: 'src/info/index.js',
  },
  plugins: [
    new MiniCssExtractPlugin({
      filename: '[name].[chunkhash].css',
    })
  ]
}

MiniCssExtractPlugin would output the following:

  • home.126a82893132ed13751e.css
  • home.126a82893132ed13751e.js
  • info.3fa7ef8e382a920a42bc.css
  • info.3fa7ef8e382a920a42bc.js

I would of expected this plugin to then create:

  • home-tablet.[chunkhash].css
  • home-desktop.[chunkhash].css
  • info-tablet.[chunkhash].css
  • info-desktop.[chunkhash].css

But instead it only outputs:

  • index-tablet.[chunkhash].css
  • index-desktop.[chunkhash].css

Assistance would be greatly appreciated.

davidhouweling avatar Jan 16 '19 02:01 davidhouweling

Hi @davidhouweling

thanks for reporting this!

you're right, it should use the entry name for the extracted CSS and not the imported filename – I'll try to fix it as soon as I've some time

SassNinja avatar Jan 16 '19 07:01 SassNinja

I did try and debug it locally today but couldn't work out how to correctly map from the plugin via interpretName and then onward.

davidhouweling avatar Jan 16 '19 08:01 davidhouweling

how to correctly map from the plugin via interpretName

You mean interpolateName, don't you? (I've no idea what interpretName is)

However I know the reason for the issue now: it's how I determine the filename for the extracted files in my loader.

options.basename = interpolateName(this, '[name]', {});

This does correctly take the name index of the imported index.scss. Unfortunately I haven't found a way yet to get the entry name within the loader. According to @sokra it's not possible: https://github.com/webpack/webpack/issues/6124

A workaround would be an additional option in my plugin to use the folder name instead of the filename (would solve your problem). But I don't like this hack and it's still not identically to the mini-css-extract plugin which definitely takes the entry name (I've tested it by using different entry name than folder name).

I guess I have to dig deeper into the mini-css-extract plugin to understand how they are doing it. Will definitely take same time since it's not a simple plugin...

If you wanna join the researching – any help is appreciated

SassNinja avatar Jan 16 '19 13:01 SassNinja

Yes I did, sorry i was typing my response from my phone rather than my PC. I'll try and do some research on my end looking into mini-css-extract-plugin as well. If i make progress I'll raise a PR.

davidhouweling avatar Jan 17 '19 00:01 davidhouweling

after digging through mini-extract-css-plugin and reading webpack documentation I believe the implementation is quite different (though it's quite difficult to understand given the amount of internal webpack plugins it uses).

From my rough understanding of it, mini-extract-css-plugin's loader doesn't actually manipulate anything (it just exports a noop for default) as it uses the pitching loader ability.

At least from what I can deduce, the loader is used primarily to keep track of dependencies and clean out the current individual files from the compilation.assets. The dependency factories look to be loaded from the plugin first though, which is how the loader tells the plugin what files to work with (uncertain on this).

Then the plugin itself does the heavy lifting of extracting everything out. One key thing is that mini-extract-css-plugin does not work with style-loader.

media-query-plugin on the other hand wants to work with style-loader and for valid reasons I understand. However, I think this convolutes things.

It might be best to separate the usage. If a person wishes to use it with style-loader, the plugin itself isn't actually necessary. In the case of using it with mini-css-extract-plugin, ultimately the implementation could be done without the loader at all. The implementation in this case should simply hook in at additionalAssets (like you presently have), and scan through all the present assets for the CSS files and perform the tweaks accordingly. Though the plugin would probably need to be modified to track the entry point name to the end file name so that the plugin options continue to work?

davidhouweling avatar Jan 22 '19 00:01 davidhouweling

Many thanks for your research!

though it's quite difficult to understand given the amount of internal webpack plugins it uses

yup, I was facing the same problem

One key thing is that mini-extract-css-plugin does not work with style-loader.

I guess I haven't seen it like this yet but you're right, I'm trying to support more than the mini-css-extract plugin although there's hardly any need of media query extraction if using the style-loader (mostly in dev mode).

the implementation could be done without the loader at all

I decided to use a loader because it saved me work with the sourcemaps. If I do all the work within the plugin I need to take care of all included CSS files (source and the emitted ones). But seems there's probably no other way to access the entry name.

So the bottom line is: rework of the whole plugin. Unfortunately I've not that much time at the moment but any PR is welcome and I'd be glad to support.

SassNinja avatar Jan 23 '19 09:01 SassNinja

Unfortunately I've had to move on to other work as well but if a bit of time opens up for me i'll look to what I can do to help.

What are your thoughts on dropping the loader all together and doing everything through the plugin? Though I guess if you wanted to continue to support source maps you would have to do very similarly to mini-extract-css-plugin with tracking dependencies, and all.

davidhouweling avatar Jan 23 '19 23:01 davidhouweling

What are your thoughts on dropping the loader all together and doing everything through the plugin?

In general I'm a friend of getting rid of the loader so that users only need to include one plugin. But maybe a loader will be still necessary similar to mini-css-extract-plugin.

Besides easy-to-use I also want a clean code that's easy to maintain. I'm afraid you're right and have to do all the dependency tracking myself – no idea how much work this will be in the end.

Anyway sourcemaps is sth that mustn't get lost when reworking the plugin.

SassNinja avatar Jan 24 '19 07:01 SassNinja

If you're happy to drop style-loader support, and given mini-css-extract-plugin keeps track of its dependencies and generates the correct source map (I believe), wouldn't it be doable to simply chain on from there? It does make it that the plugin directly relies on min-css-extract-plugin, which I think it does anyway?

davidhouweling avatar Jan 25 '19 02:01 davidhouweling

wouldn't it be doable to simply chain on from there?

before writing my own plugin I actually thought abt extending the mini-css-extract-plugin – without success https://github.com/webpack-contrib/mini-css-extract-plugin/issues/162

I agree my plugin is (more or less) made for using it together with mini-css-extract-plugin but since it's an individual plugin now I'd prefer to keep it as much independent as possible (if this doesn't mean a huge amount of additional work) so you may theoretically use it with other loaders (e.g. extract-loader) as well

will try to go through my code and the mini-css-extract-plugin as soon as I'm not that busy anymore.

Unfortunately I've had to move on to other work as well but if a bit of time opens up for me i'll look to what I can do to help.

Sure I can definitely understand this. In case you find any time for it feel free to share your thoughts and/or (intermediate) result. Could be helpful to me or any other volunteer who wanna jump in.

SassNinja avatar Jan 28 '19 13:01 SassNinja

trying to figure out '[name]-desktop.css' but don't know how to 🤦🏻‍♂️can you please help?

new MiniCssExtractPlugin({
  filename: '[name].css',
}),

new MediaQueryPlugin({
  include: [
    'PLPPage',
    'PDPPage',
  ],
  queries: {
    '(min-width: 769px)': 'desktop',
  },
}),

Thanks

w3debugger avatar Jan 22 '20 20:01 w3debugger

I'm seeing a similar issue when using the following patterns in min-css-extract-plugin:

filename: 'index.[contenthash:8].css'

This results in only 2 files, even though I have 3 distinct queries defined in this plugin:

index.1fa2f0d8.css
index.[contenthash:8].css

There are a few issues:

  1. I expect the second file to be named something like index-desktop.12345678.css (that is, [contenthash:8] gets properly replaced.
  2. There should be 4 files, right? 1 base and 3 breakpoints?
  3. I can't get the include option to work. I've passed it the name of the chunk (['main']), the name of the output file (['index']), but nothing worked. I had to resort to true. (This might be a separate issue, but I'm including it here in case it informs the other problems.)
  4. None of these variants are showing up in the main chunk of my webpack stats object. Here's the stats object received by webpack-stats-plugin's transform function:
{
  assetsByChunkName: {
    main: [
      'index-legacy.1fa2f0d8.css',
      'index-legacy.3fe56758.js',
      'index-legacy.3fe56758.js.map'
    ],
    'App-desktop': 'index-legacy.[contenthash:8].css',
    '_categories-section-desktop': 'index-legacy.[contenthash:8].css',
    '_hero-section-desktop': 'index-legacy.[contenthash:8].css',
    '_ihd-section-desktop': 'index-legacy.[contenthash:8].css',
    '_partner-section-desktop': 'index-legacy.[contenthash:8].css',
    '_stats-section-desktop': 'index-legacy.[contenthash:8].css'
  }
}
{
  assetsByChunkName: {
    main: [
      'index.1fa2f0d8.css',
      'index.573e07e5.js',
      'index.573e07e5.js.map'
    ],
    'App-desktop': 'index.[contenthash:8].css',
    '_categories-section-desktop': 'index.[contenthash:8].css',
    '_hero-section-desktop': 'index.[contenthash:8].css',
    '_ihd-section-desktop': 'index.[contenthash:8].css',
    '_partner-section-desktop': 'index.[contenthash:8].css',
    '_stats-section-desktop': 'index.[contenthash:8].css'
  }
}

Definitely looks like something is going sideways with the compile. I would instead expect a stats object similar to:

{
  assetsByChunkName: {
    main: [
      'index-legacy.1fa2f0d8.css',
      'index-legacy-mobile.12345678.css',
      'index-legacy-tablet.23456789.css',
      'index-legacy-desktop.34567890.css',
      'index-legacy.3fe56758.js',
      'index-legacy.3fe56758.js.map'
    ],
  }
}
{
  assetsByChunkName: {
    main: [
      'index.1fa2f0d8.css',
      'index-mobile.12345678.css',
      'index-tablet.23456789.css',
      'index-desktop.34567890.css',
      'index.573e07e5.js',
      'index.573e07e5.js.map'
    ],
  }
}

ZebraFlesh avatar Feb 21 '20 00:02 ZebraFlesh