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

Chunk information in missing for compressed files #185

Closed
xitter opened this issue Aug 28, 2020 · 23 comments
Closed

Chunk information in missing for compressed files #185

xitter opened this issue Aug 28, 2020 · 23 comments

Comments

@xitter
Copy link

xitter commented Aug 28, 2020

When a compressed file is emitted, the chunks do not get updated. i.e. for a test.js file, chunk information looks like:

compilation.chunks = [
    { files: ['test.js']}
]

Due to this issue, the final stats in thee done hook, do not map chunks to compressed assets.

  • Operating System: macOS catalina
  • Node Version: 10
  • NPM Version:
  • webpack Version: 4 & 5
  • compression-webpack-plugin Version: ^5.0.1

Expected Behavior

As a new compressed asset is being added, chunks should also have reference to this new asset, like:

compilation.chunks = [
    { files: ['test.js', 'test.js.gz']}
]

Actual Behavior

compilation.chunks = [
    { files: ['test.js']}
]

Code

Webpack config used:

   output: {
        path: path.resolve('build/test'),
        filename: '[name].js',
    },
    plugins: [
        new CompressionPlugin({
            filename: '[path].gz',
            algorithm: 'gzip',
            test: /\.(js|jsx|css|html|png|svg|jpg|gif)$/,
            threshold: 10240,
            minRatio: 0.8,
        }),
    ],

How Do We Reproduce?

Try out the above config and check the stats in done hook like:

compiler.hooks.done.tapAsync('stats-test', (stats, cb) => {
   console.log(stats.toJSON());
   cb();
});
@xitter xitter mentioned this issue Aug 28, 2020
@alexander-akait
Copy link
Member

You should use assetsInfo to get information https://github.com/webpack-contrib/compression-webpack-plugin/blob/master/src/index.js#L155

@alexander-akait
Copy link
Member

alexander-akait commented Aug 28, 2020

But for webpack@4, you need to put own plugin after compression-webpack-plugin, for webpack@5 it is fixed

@xitter
Copy link
Author

xitter commented Aug 28, 2020

Doesn't look fixed in v4 or v5, getting below output, see chunkNames: [], no chunks identified for the compressed asset.

Output of console.log(stats.toJSON());:

Screenshot 2020-08-28 at 6 42 42 PM

To fix this, chunks object need to be updated to have new asset name as well.

i.e. { files: ['test.js', 'test.js.gz']}

I will send a PR to address this.

@alexander-akait
Copy link
Member

What do you mean { files: ['test.js', 'test.js.gz']}? we have related https://github.com/webpack-contrib/compression-webpack-plugin/blob/master/src/index.js#L234

What is webpack version?

@alexander-akait
Copy link
Member

It is asset, so we don't know is this chunk or just asset

@xitter
Copy link
Author

xitter commented Aug 28, 2020

Webpack version is 5. related won't help as related is attached to the original asset's info.

See the chunks[0].files array in the above image, it just contains testApp.js, and should contain testApp.js.gz as well. Because this chunk is part of gzipped file as well.

Due to the gzipped file not present in this chunk's file array, webpack's stat generation logic(L260) thinks that gzipped file has no chunks, and outputs assets[0].chunks(see in above image) as an empty array. Expected is that assets[0].chunks should be ['testApp'], as this asset has that chunk.

@alexander-akait
Copy link
Member

Because test.js.gz is not chunk, it is asset, why do you need it in files?

@alexander-akait
Copy link
Member

If copy-webacpk-plugin emit js asset, it is just asset, not chunk, so you fix is invalid

@xitter
Copy link
Author

xitter commented Aug 28, 2020

You are right, test.js.gz is an asset and is related to a chunk. files is property of chunk, which indicates that which files have this chunk.

@xitter
Copy link
Author

xitter commented Aug 28, 2020

copy-webpack-plugin doesn't operate on the files generated from the build process, so it need not to worry about the chunk of the build process.

@alexander-akait
Copy link
Member

alexander-akait commented Aug 28, 2020

@xitter in this case files should contain only js files, not gzip or something else, it is big breaking change

@xitter
Copy link
Author

xitter commented Aug 28, 2020

Including this can make other plugins' life easy to see the correct stat info. See discussion on webpack-contrib/webpack-bundle-analyzer#377 (comment)

@alexander-akait
Copy link
Member

@xitter Why do not use assetInfo? We have related property for this

@alexander-akait
Copy link
Member

Let's open to find the most suitable and convenient solution to the problem

@xitter
Copy link
Author

xitter commented Aug 28, 2020

@evilebottnawi I see 2 issues with related property in assetInfo,

  1. related property is available in the original asset. For the compressed asset, assetInfo is {compressed: true}.
  2. Even if we solve 1st by adding related property in compressed asset's info, webpack's stat generation logic(L266-L283) isn't aware of related property and only looks for chunk files to determine which assets are part of which chunks.

So I see 2 solutions:

A. Add a related property for compressed asset as well. This can be read by other plugins to compute the chunk list on their own.
B. Update the chunk files in this plugin itself(#186). So webpack's stat generation takes care of determining chunks correctly.

The only caveat with A is that other plugins have to be aware of related property, and do this stat generation on their own.

@alexander-akait
Copy link
Member

alexander-akait commented Aug 29, 2020

A. Add a related property for compressed asset as well. This can be read by other plugins to compute the chunk list on their own.

No, we can't because the related property used by webpack for deleting assets, compluation.deleteAsset() remove assets and related files, if we added the related property to compressed files it breaks logic

B. Update the chunk files in this plugin itself(#186). So webpack's stat generation takes care of determining chunks correctly.

webpack already generate stats in right way, compressed assets is not chunk, it is asset as I written above

Even if we solve 1st by adding related property in compressed asset's info, webpack's stat generation logic(L266-L283) isn't aware of related property and only looks for chunk files to determine which assets are part of which chunks.

Why do not improve logic? It should be very easy, just check asset info.

@alexander-akait
Copy link
Member

@xitter friendly ping

@xitter
Copy link
Author

xitter commented Sep 2, 2020

Why do not improve logic? It should be very easy, just check asset info.

If the asset is deleted(with deleteOriginalAsset), there is no asset info left. And stats are empty as the original asset is not available, while chunks keep referring to that.

@alexander-akait
Copy link
Member

@xitter I don't understand your problem, please clarify

@xitter
Copy link
Author

xitter commented Sep 2, 2020

Below is the stat information for config:

new CompressionPlugin({
            filename: '[path].gz',
            algorithm: 'gzip',
            test: /\.(js|jsx|css|html|png|svg|jpg|gif)$/,
        }),

Stats:

  "assets": [
    {
      "name": "testApp.js",
      "size": 4392,
      "chunks": [
        "testApp"
      ],
      "chunkNames": [
        "testApp"
      ],
      "info": {
        "related": {
          "gziped": "testApp.js.gz"
        }
      },
      "emitted": true
    },
    {
      "name": "testApp.js.gz",
      "size": 1475,
      "chunks": [],
      "chunkNames": [],
      "info": {
        "compressed": true
      },
      "emitted": true
    }
  ],

This has the assetInfo as you mentioned.

However, if we use deleteOriginalAssets: true option, stat output looks like below:

  "assets": [
    {
      "name": "testApp.js.gz",
      "size": 1475,
      "chunks": [],
      "chunkNames": [],
      "info": {
        "compressed": true
      },
      "emitted": true
    }
  ],
  "chunks": [
    {
      "id": "testApp",
      "rendered": true,
      "initial": true,
      "entry": true,
      "size": 18,
      "names": [
        "testApp"
      ],
      "files": [
        "testApp.js"
      ],
      "hash": "f185ec480e2041373dc2",
      "siblings": [],
      "parents": [],
      "children": [],
      "childrenByOrder": {},
      "origins": [
        {
          "module": "",
          "moduleIdentifier": "",
          "moduleName": "",
          "loc": "testApp",
          "request": "./products/test/src/index.jsx",
          "reasons": []
        }
      ]
    }
  ],

Here, this assetInfo can not be used to map chunks as chunks are referring to testApp.js(original asset), but that's not in the assets array.

@alexander-akait
Copy link
Member

alexander-akait commented Sep 2, 2020

Just improve this, add additional check in your code, if you need we can added { originalSize } to asset info, what do you need? what information?

@alexander-akait
Copy link
Member

@xitter friendly ping

@alexander-akait
Copy link
Member

Closing due to inactivity. Thanks!

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