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

Use webpack's performance.hints for reporting non-fatal errors #1380

Closed
jeffposnick opened this issue Mar 20, 2018 · 8 comments
Closed

Use webpack's performance.hints for reporting non-fatal errors #1380

jeffposnick opened this issue Mar 20, 2018 · 8 comments
Assignees
Labels
Developer Experience Related to ease of use for developers. workbox-webpack-plugin

Comments

@jeffposnick
Copy link
Contributor

Library Affected:
workbox-webpack-plugin

Issue or Feature Request Description:
workbox-webpack-plugin v3 introduced a model in which all assets in the webpack compilation are potentially eligible for precaching (with the include/exclude/chunks/excludeChunks options available for modifying the defaults).

Because of this new model, it's (usually) not necessary to use the previous glob-based configuration options, which matched files in the output filesystem, rather than matching against assets in the webpack compilation. But there are still some scenarios in which they might be necessary, so we didn't remove support, and documented them at https://developers.google.com/web/tools/workbox/modules/workbox-webpack-plugin#cache_additional_non-webpack_assets

If you do use those glob-based options in v3, you end up generating an additional list of precached URLs, and options like manifestTransforms and dontCacheBustUrlsMatching only apply to that additional list, not the list derived from the webpack asset pipeline.

We should explicitly add one of two possible messages to compilation.warnings to let folks know about this scenarios. They are not always due to mistakes, so there might be some false positives, but if that's the case, it's easy enough to ignore the warning.

  • When globPatterns or any of the related options is used, warn that it's often not needed, and link to the relevant docs.
  • When manifestTransforms or any of the related options that modify the glob-derived manifest is used, and globPatterns is not used, warn that those options will have no effect (because there is no glob-derived manifest) and link to the relevant docs.
@freezy
Copy link

freezy commented Mar 20, 2018

Might be also worth noting in the migration guide, since this seems like a breaking change to me.

@jeffposnick
Copy link
Contributor Author

It is a breaking change, yup. The migration guide currently reads:

The plugin has been substantially rewritten, and in many cases, can be used in a "zero-configuration" mode. Consult the documentation for the updated API surface.

...snip...

  • By default, assets in the webpack compilation pipeline will be precached, and it is no longer necessary to configure globPatterns. The only reason to continue using globPatterns is if you need to precache assets that are not included in your webpack build. In general, when migrating to the v3 plugin, you should start by removing all of your previous glob-based configuration, and only re-add it if you specifically need it.

We've iterated on that language a bit, and I hope the current wording steers developers in the right direction.

@freezy
Copy link

freezy commented Mar 20, 2018

Well maybe put the actual options in whose behavior changed. I usually Ctrl+F against the options I'm using and if nothing turns up I assume nothing changed.

@glen-cheney
Copy link

Since this warning cannot be hidden, is runtime-caching preferred for assets not controlled by webpack?

@jeffposnick
Copy link
Contributor Author

jeffposnick commented Mar 23, 2018

Setting globPatterns is still a supported use case for developers using the webpack plugin who need to precache assets not controlled by webpack (in addition to precaching webpack-controlled assets).

The new warning message is there in v3.0.1 because we saw a number of developers confused and under the impression that globPatterns was required, which could happen in particular when migrating from v2.

Is the presence of the warning breaking anything in your build, or do you just find it annoying? I don't know of a clean way of opting-out of that warning message for developers who "know what they're doing" and really do need to continue using globPatterns.

@jeffposnick jeffposnick reopened this Mar 23, 2018
@glen-cheney
Copy link

Thanks for the quick reply!

It's not breaking anything, but I would like to be able to hide it since I now understand the difference between the webpack-controlled assets and others that I want to pre-cache.

Webpack lets me hide some other warnings, like the performance ones, by setting performance: { hints: false }. Docs.

@jeffposnick
Copy link
Contributor Author

Thanks for passing along the prior art around configuring webpack to hide certain warnings. Seems like we could do something similar. I'll leave this issue open to track that.

@jeffposnick jeffposnick changed the title Warn when glob-specific options are used in webpack config Use webpack's performance.hints for reporting non-fatal errors Aug 3, 2018
@jeffposnick
Copy link
Contributor Author

We're currently reporting things as per the recommended wepback approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Related to ease of use for developers. workbox-webpack-plugin
Projects
None yet
Development

No branches or pull requests

3 participants