-
Notifications
You must be signed in to change notification settings - Fork 816
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
Fix for runtimeCaching plugin config conversion #1335
Conversation
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.
Thanks for this Jeff.
switch (pluginName) { | ||
// Special case logic for plugins that have a required parameter, and then | ||
// an additional optional config parameter. | ||
case 'backgroundSync': { |
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 was thinking of combining this and the next case
, which would prevent some code duplication. One reads the required value from the name
parameter, and the other reads it from channelName
, though, so there would be some extra logic that would need to be added.
I ended up keeping them separate, but if folks would prefer them combined, I could do that.
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 think seperate makes sense - makes the tests super easy to read.
PR-Bot Size PluginChanged File Sizes
New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin☠️ WARNING ☠️We are using 155% of our max size budget. Total Size: 22.7KB Gzipped: 9.1KB |
R: @addyosmani @gauntface
Fixes #1334
With additional tests as well.