GoogleChromeLabs / tooling.report

tooling.report a quick way to determine the best build tool for your next web project, or if tooling migration is worth it, or how to adopt a tool's best practice into your existing configuration and code base.
https://tooling.report
Apache License 2.0
848 stars 49 forks source link

fix avoid-cascade testcase #371

Closed sokra closed 4 years ago

sokra commented 4 years ago

This fixes a few problems with the avoid-cascade test cases for rollup and webpack.

The description claims that the import-map is uncachable, but this can't be the best practice, considering that it's easily possible to make it cacheable for rollup and it's already cacheable by default for webpack. So it changed the description and updated the rollup plugins to avoid inlining the import-map and add a hash instead.

The example code never used an asset in an on-demand loaded part of the application. Without that webpack would just pass the test with the defaults (initial chunks do not have hashes in the runtime chunk, so it won't case a cascade). I added another asset in an on-demand-loaded chunk to make the test case more difficult (and more real worldy). Now webpack would really fail this test case with the defaults. I added a few lines of description to explain why this is the default. (TL'DR: in larges apps # of assets >> # of chunks, so we go with one-level cascade to allow loading asset hashes on demand and avoid a huge import-map)

Anyway webpack allows to configure caching behavior for each module with the splitChunks.cacheGroups configuration and you can move the asset hashes anywhere you like to, e. g. into the runtime...

googlebot commented 4 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

sokra commented 4 years ago

@googlebot I signed it!

googlebot commented 4 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

googlebot commented 4 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

jakearchibald commented 4 years ago

It seems to work. I'm not sure if all these config switches are actually documented to do the things they end up doing.

developit commented 4 years ago

Just double checking something here: I think my wording may have been misleading regarding "uncacheable" - what I meant was that the mapping (in webpack's case runtime.js) is not permanently cacheable, since it doesn't have a hash. Perhaps that's moot though, since it could be hashed and referenced with that hash from the HTML document that then becomes the time-based invalidator?

jakearchibald commented 4 years ago

Thanks @sokra and @developit!