-
Notifications
You must be signed in to change notification settings - Fork 822
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
Support for offlineGoogleAnalytics config in workbox-build config #1610
Conversation
How about you can pass |
It's historically been messy when converting from an #1598 just took care of that for one case, when folks might end up setting offlineGoogleAnalytics: {
hitFilter: (params) => {
const queueTimeInSeconds = Math.round(params.get('qt') / 1000);
params.set('cm1', queueTimeInSeconds);
},
}, Do you feel like enough folks end up needing custom configs that the (slight) extra complexity is worthwhile? Might as well? |
I think it's worth it, if it's possible. When not using any customization options, users have no way of knowing if their offline analytics are working, so I'd like to encourage more usage of the customization options wherever possible. |
Okay, I've used the technique from #1598 and passing in a full range of config options, including functions, seems to work. |
PR-Bot Size PluginChanged File SizesNo file sizes have changed. New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin9.76KB gzip'ed (65% of limit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
R: @philipwalton
CC: @hoten
Fixes #1568
The name of the new boolean config property (defaulting to
false
) isofflineGoogleAnalytics
, and it's being added togenerateSW
andgenerateSWString
modes ofworkbox-build
. It could also be used in the wrappers aroundworkbox-build
, likeworkbox-cli
andworkbox-webpack-plugin
.A custom config would still require either using
importScripts
to pull in a separate file while ingenerateSW
mode, or switching toinjectManifest
mode. But for the basic functionality, where all you want isthis should be sufficient.
Assuming this PR looks good, we'll need to get https://developers.google.com/web/tools/workbox/modules/workbox-build#full_generatesw_config updated once we release this, as part of what will presumably be Workbox
v3.5.0
.