aurelia / v1

The Aurelia 1 scaffolding repo used by our tools to setup new projects.
MIT License
9 stars 3 forks source link

fix(webpack) correct MiniCssExtractPlugin fix: #18 #19

Closed avrahamcool closed 4 years ago

3cp commented 4 years ago

@Sayan751 can you help to review?

Sayan751 commented 4 years ago

@avrahamcool Thanks for the PR. Can you please add some details, as why the previous configuration was problematic?

avrahamcool commented 4 years ago

@Sayan751 Already did in issue #18

Sayan751 commented 4 years ago

@avrahamcool @3cp Although I fundamentally don't see any problem with the current PR, I could not unfortunately reproduce the issue. It would be nice to see the reproduction of the original issue.

avrahamcool commented 4 years ago

I'll create a small repo for that.

Sayan751 commented 4 years ago

Thanks @avrahamcool , that would be really helpful!

avrahamcool commented 4 years ago

Thanks @avrahamcool , that would be really helpful!

@Sayan751 https://github.com/avrahamcool/webpack-bundling-problem

Sayan751 commented 4 years ago

@avrahamcool Thanks for the repo. This has helped me understanding the issue. Also found this, which explains the root cause of the issue.

@3cp The proposed change looks good to me.

For completeness, let me say that there is another alternative. I was also playing with the webpack options. IMO it is not that bad to classify the assets into different directories like css, images etc. as compared to a heap of garbage in the dist dir. From that perspective, I tried something like the following (abbreviated).

{
  module: {
    rules: [
      {
        test: /\.css$/i,
        issuer: [{ not: [{ test: /\.html$/i }] }],
        use: extractCss ? [{
          loader: MiniCssExtractPlugin.loader,
          options: {
            publicPath: '../'                                 // <-- L1.1
          }
        }, ...cssRules
        ] : ['style-loader', ...cssRules]
      },
      {
        test: /\.(png|gif|jpg|cur)$/i,
        loader: 'url-loader',
        options: {
          limit: 8192,
          name: 'images/[hash].[ext]'                         // <-- L2
        }
      },
    ]
  },
  plugins: [
    new MiniCssExtractPlugin({
      filename: production ? 'css/[name].[contenthash].bundle.css' : 'css/[name].[hash].bundle.css',      //<--  L1.2
      chunkFilename: production ? 'css/[name].[contenthash].chunk.css' : 'css/[name].[hash].chunk.css'    //<--  L1.3
    })
  ]
}

Note that in L1.2, and L1.3 we are pushing the extracted css files to the css directory. Now if we provide the relative path to the "root" as done in L1.1 using the publicPath the MiniCssExtractPlugin will prefix the urls with that. This allow us to put the images in a different directory named images as done in L2. The generated dist structure will be bit more cleaner I believe with this, as well as correcting the generated URLs.

3cp commented 4 years ago

Thank you both!