webpack-contrib / compression-webpack-plugin

Prepare compressed versions of assets to serve them with Content-Encoding
MIT License
1.41k stars 108 forks source link

Using two instances of CompressionPlugin causes files to be unintentionally deleted? #245

Open tomkelsey opened 3 years ago

tomkelsey commented 3 years ago

Expected Behavior

Using Webpack 4 and version 6 of this plugin the output was as expected - a subdirectory with brotli compressed files and then the "original" files replaced with gzipped versions.

Actual Behavior

After upgrading to Webpack 5 and version 7, I was getting this error:

Conflict: Multiple assets emit different content to the same filename

And so I tried adding deleteOriginalAssets: true which fixed the error but it resulted in no brotli subdirectory being outputted.

I did some digging and it appears the brotli files are emitted but, at a guess, they then get deleted? Perhaps something to do with related assets and how webpack handles everything?

Code

I'm not familiar with the inner workings of webpack or it's plugins but I managed to bodge together a solution whereby the compression plugin supports multiple algorithms with one instance, that way you can choose to only invoke the deleteOriginalAssets logic if you're on the final pass and have already finished every other compression. The code is here: https://github.com/tomkelsey/compression-webpack-plugin/tree/multiple-algorithms I didn't do a PR as it's definitely not up to scratch and feel like there's probably a much more straightforward solution to this problem!

How Do We Reproduce?

alexander-akait commented 3 years ago

Please do not ignore How Do We Reproduce?, we have tests on this cases

tomkelsey commented 3 years ago

Hi - sorry - I'm not familiar with the test framework or webpack so hoped sharing the issue, my written explanation and my workaround was better than saying nothing at all.

That said - I appreciate it makes it a lot more difficult for someone to verify the issue or debug it.

With that in mind I've had a go at hacking something together (sorry if I've got the wrong end of the stick!):

  it("should work with multiple plugins and using same filename", async () => {
    const compiler = getCompiler(
      "./entry.js",
      {},
      {
        output: {
          path: `${__dirname}/dist`,
          filename: "[name].js?var=[hash]",
          chunkFilename: "[id].[name].js?ver=[hash]",
        },
      }
    );

    new CompressionPlugin({
      algorithm: "brotliCompress",
      filename: "[path]/br/[file]",
    }).apply(compiler);
    new CompressionPlugin({
      algorithm: "gzip",
      filename: "[file]",
      deleteOriginalAssets: true,
    }).apply(compiler);

    const stats = await compile(compiler);

    console.log(getAssetsNameAndSize(stats, compiler));

    expect(getAssetsNameAndSize(stats, compiler)).toMatchSnapshot("assets");
    expect(getWarnings(stats)).toMatchSnapshot("warnings");
    expect(getErrors(stats)).toMatchSnapshot("errors");
  });

I believe you'll see from the console log that there's no br subdirectory.

If you remove

new CompressionPlugin({
      algorithm: "gzip",
      filename: "[file]",
      deleteOriginalAssets: true,
    }).apply(compiler);

You should see that the br subdirectory is created

alexander-akait commented 3 years ago

You need to use different filename otherwise you will create multiple assets with the same name

alexander-akait commented 3 years ago

I.e. you will have multiple files with the same name https://github.com/webpack-contrib/compression-webpack-plugin/blob/master/src/index.js#L27

tomkelsey commented 3 years ago

Hi,

Thanks for the speedy responses!

For the setup I'm using elsewhere I ideally need to keep the filename the same but with a different path - perhaps it was by happy coincidence or fluke but it worked as I'd hoped on an earlier version of the plugin and with Webpack 4.

The filename option provided to CompressionWebpackPlugin is different - "[path]/br/[file]" vs "[file]", but I'm guessing you're referring to the filename itself?

My rather elaborate workaround (https://github.com/tomkelsey/compression-webpack-plugin/tree/multiple-algorithms) appears to solve the specific issue I'm facing but didn't know if there was a more straightforward solution.

alexander-akait commented 3 years ago

hm, maybe bug, can you send broken test?

tomkelsey commented 3 years ago

Hey - sorry if I'm misunderstanding but the test I shared above highlights the issue I believe.

I think the output of getAssetsNameAndSize() should include multiple files in the /br/ subdirectory but it only shows the gzipped files.

it("should work with multiple plugins and using same filename", async () => {
    const compiler = getCompiler(
      "./entry.js",
      {},
      {
        output: {
          path: `${__dirname}/dist`,
          filename: "[name].js?var=[hash]",
          chunkFilename: "[id].[name].js?ver=[hash]",
        },
      }
    );

    new CompressionPlugin({
      algorithm: "brotliCompress",
      filename: "[path]/br/[file]",
    }).apply(compiler);
    new CompressionPlugin({
      algorithm: "gzip",
      filename: "[file]",
      deleteOriginalAssets: true,
    }).apply(compiler);

    const stats = await compile(compiler);

    console.log(getAssetsNameAndSize(stats, compiler));

    expect(getAssetsNameAndSize(stats, compiler)).toMatchSnapshot("assets");
    expect(getWarnings(stats)).toMatchSnapshot("warnings");
    expect(getErrors(stats)).toMatchSnapshot("errors");
  });
alexander-akait commented 3 years ago

Yep, bug

alexander-akait commented 3 years ago

Try to fix it in near future

alexander-akait commented 3 years ago

When you use deleteOriginalAssets related assets deleted too (source maps, other gzip/compressed files/etc, here problem), because if you don't need original files you don't need related, for popular case we have keep-source-map, i.e. remove all related assets except source maps.

But you case is more edge :smile: here two solutions: use filename: "[path]/js/[file]", or we can implement deleteOriginalAssets: (relatedFiles) => {} where you will decide what files should you keep, it is really edge case, in normal workflow I strong recommend avoid override assets, it can lead to weird behavior, especial for non official plugins

alexander-akait commented 3 years ago

In theory you need deleteOriginalAssets: 'keep-other-compressed-files', but here problem, we don't know how to detect it, other plugins also can create different files and we don't know is it compressed or not (in theory we can add more information for assets), but this will create compatibility problems with other plugins and it is ambiguous.

alexander-akait commented 3 years ago

I would call it a limitation. Will be great if you can use filename: "[path]/js/[file]", otherwise I will implement function and you will need to decide which files to keep/delete, maybe be some tricky if you have non official plugins

tomkelsey commented 3 years ago

Hi,

Thanks for looking into this.

I've realised the "solution" I linked above using multiple algorithms with one instance of the plugin doesn't work properly. Webpack seems to be losing track of what files had been deleted/created (which fits with the potential weird behaviour you mentioned above).

I've taken your advice and now use "[path]/br/[file]" and "[path]/gz/[file]" for the two filenames.

I've then pushed the problem to another plugin (https://github.com/MikaAK/s3-plugin-webpack) which I've rejigged so that it uploads the /br/ files into the /br/ directory but the /gz/ files into the base directory.

Thanks again for your efforts in trying to resolve this!

alexander-akait commented 3 years ago

Webpack seems to be losing track of what files had been deleted/created (which fits with the potential weird behaviour you mentioned above).

There is no webpack problems, it is hard for us, I can implement this, but it is not easy (also it reduce performance), in some cases limitation is good :smile: