-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
feat: Use the Workbox webpack plugin in pwa template #769
Conversation
.plugin('workbox') | ||
.use(workboxWebpackModule[workboxPluginMode], [workBoxConfig]) | ||
} else { | ||
throw new Error(`${workboxPluginMode} is not a supported Workbox webpack plugin mode. ` + |
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.
I wasn't sure what the philosophy was around misconfiguration. If there's something that should be done here other than throwing an Error
, I'm happy to change.
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.
Since this is a built-in plugin, you can add it the user options validation schema here: https://github.com/vuejs/vue-cli/blob/dev/packages/%40vue/cli-service/lib/options.js#L3-L35
But throwing an error is also fine.
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.
I ❤️ joi
, but have found the default error messages to be a little hard to read, so I'll just stick with a custom Error
for this specific failure.
Thanks @jeffposnick ! Yes, we will be documenting each plugin in its own README, but we haven't started finalizing the format yet. For now, feel free to provide any information you feel necessary, I'll be working on docs next week and revise it to make sure it's consistent. |
new RegExp('\.map$'), | ||
new RegExp('img/icons/'), | ||
new RegExp('favicon\.ico$'), | ||
new RegExp('manifest\.json$') |
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.
Looks like the default exclude is [/\.map$/, /^manifest.*\.js(?:on)?$/]
- any reason to exclude the icons and favicon as well?
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.
The request that a browser makes for either a registered favicon or for the home screen icon bypasses the service worker, so there's no benefit from caching them.
Thanks! I'll add some docs to the README and push an additional commit. |
PTAL. |
Thanks! 🎉 |
@jeffposnick I noticed that even in the production build, the service worker pulls a dev version of workbox from the CDN. Is there anyway to configure which CDN link is used? |
Aha! I noticed it fetches the dev/prod version based on whether it's on localhost or not. Very nice! |
(The dev/prod dynamic loading is all thanks to @gauntface!) |
R: @yyx990803 & co.
See #717 for context.
Open question: should I also add something to the
README.md
for the template explaining which configuration options are supported, and how they map to the Workbox options? If so, is there an example of another template that has this written up in the format you're looking for?