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

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

Open
tomkelsey opened this issue Mar 31, 2021 · 14 comments · May be fixed by #246
Open

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

tomkelsey opened this issue Mar 31, 2021 · 14 comments · May be fixed by #246

Comments

@tomkelsey
Copy link

  • Operating System: OSX 11.2.2
  • Node Version: v12.14.1
  • NPM Version: 6.13.4
  • webpack Version: 5.28.0
  • compression-webpack-plugin Version: 7.1.2

Expected Behavior

  • Use Compression Plugin once to compress with brotli (same file name, different path - filename: '[path]/br/[file]'
  • Use Compression Plugin again to compress as gzip (same file name, same path - filename: '[file]')

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
Copy link
Member

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

@tomkelsey
Copy link
Author

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
Copy link
Member

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

@alexander-akait
Copy link
Member

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
Copy link
Author

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
Copy link
Member

hm, maybe bug, can you send broken test?

@tomkelsey
Copy link
Author

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
Copy link
Member

Yep, bug

@alexander-akait
Copy link
Member

Try to fix it in near future

@alexander-akait alexander-akait linked a pull request Apr 1, 2021 that will close this issue
6 tasks
@alexander-akait
Copy link
Member

alexander-akait commented Apr 1, 2021

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 😄 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
Copy link
Member

alexander-akait commented Apr 1, 2021

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
Copy link
Member

alexander-akait commented Apr 1, 2021

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
Copy link
Author

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
Copy link
Member

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 😄

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

Successfully merging a pull request may close this issue.

2 participants