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.

Sign up for GitHub

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

Revamp the runtime caching option conversion. #1160

Merged
merged 1 commit into from
Jan 5, 2018
Merged

Revamp the runtime caching option conversion. #1160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions infra/testing/validator/service-worker-runtime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function setupSpiesAndContext() {
// To make testing easier, return the name of the strategy.
strategies: {
cacheFirst: sinon.stub().returns('cacheFirst'),
networkFirst: sinon.stub().returns('networkFirst'),
},
};

Expand All @@ -47,6 +48,7 @@ function setupSpiesAndContext() {
cacheExpirationPlugin: workbox.expiration.Plugin,
cacheFirst: workbox.strategies.cacheFirst,
clientsClaim: workbox.clientsClaim,
networkFirst: workbox.strategies.networkFirst,
precacheAndRoute: workbox.precaching.precacheAndRoute,
registerNavigationRoute: workbox.routing.registerNavigationRoute,
registerRoute: workbox.routing.registerRoute,
Expand Down
21 changes: 15 additions & 6 deletions packages/workbox-build/src/entry-points/options/common-generate-schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,25 @@ module.exports = baseSchema.keys({
'staleWhileRevalidate'
)],
options: joi.object().keys({
cacheName: joi.string(),
plugins: joi.array().items(joi.object()),
cacheExpiration: joi.object().keys({
maxEntries: joi.number().min(1),
maxAgeSeconds: joi.number().min(1),
}).or('maxEntries', 'maxAgeSeconds'),
backgroundSync: joi.object().keys({
name: joi.string().required(),
options: joi.object(),
}),
broadcastCacheUpdate: joi.object().keys({
channelName: joi.string().required(),
options: joi.object(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the broadcast cache update object needs a channel name.

Copy link
Contributor Author

@jeffposnick jeffposnick Jan 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup—that requirement is enforced one line up, on line 46.

}),
cacheableResponse: joi.object().keys({
statuses: joi.array().items(joi.number().min(0).max(599)),
headers: joi.object(),
}).or('statuses', 'headers'),
cacheName: joi.string(),
expiration: joi.object().keys({
maxEntries: joi.number().min(1),
maxAgeSeconds: joi.number().min(1),
}).or('maxEntries', 'maxAgeSeconds'),
networkTimeoutSeconds: joi.number().min(1),
plugins: joi.array().items(joi.object()),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work? I.e. how does code in the node space get into the service worker? My main consideration here is that if this works, why support one way in the build process and a different way in the service worker?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code that converts whatever is configured in the node build tools into a string representation that could be inserted into a service worker file is

https://github.com/GoogleChrome/workbox/blob/v3/packages/workbox-build/src/lib/runtime-caching-converter.js

and the string is used in the template at

workbox/packages/workbox-build/src/templates/sw-template.js

Line 62 in dafc65b

<% if (runtimeCaching) { runtimeCaching.forEach(runtimeCachingString => {%><%= runtimeCachingString %><% });} %>`;

(None of that has changed recently.)

The syntax used to generate the service worker code is not exactly the same as the syntax used when executing the service worker runtime. Service worker execution implies that we're inside of the ServiceWorkerGlobalScope with the various definitions in the workbox namespace available. When the build configuration is used, we're inside of the node runtime environment, and we maintain a mapping of the keys in the config that correspond to the resulting runtime code.

I think in this PR the mapping between configuration supplied to node and resulting Workbox v3 service worker runtime code gets much closer to being 1:1, as we removed some differences that were mainly around to match sw-precache's config.

}),
}).requiredKeys('urlPattern', 'handler')),
skipWaiting: joi.boolean().default(defaults.skipWaiting),
Expand Down
2 changes: 2 additions & 0 deletions packages/workbox-build/src/lib/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,6 @@ module.exports = {
supported.)`,
'bad-runtime-caching-config': ol`An unknown configuration option was used
with runtimeCaching:`,
'invalid-network-timeout-seconds': ol`When using networkTimeoutSeconds, you
must set the handler to 'networkFirst'.`,
};
36 changes: 15 additions & 21 deletions packages/workbox-build/src/lib/runtime-caching-converter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,17 @@ function getOptionsString(options = {}) {
delete options.plugins;
}

let cacheName;
if (options.cache && options.cache.name) {
cacheName = options.cache.name;
delete options.cache.name;
}

// Allow a top-level cacheName value to override the cache.name value.
if (options.cacheName) {
cacheName = options.cacheName;
delete options.cacheName;
}

let networkTimeoutSeconds;
if (options.networkTimeoutSeconds) {
networkTimeoutSeconds = options.networkTimeoutSeconds;
delete options.networkTimeoutSeconds;
}
// Pull cacheName and networkTimeoutSeconds from the options object, since
// they are not directly used to construct a Plugin instance.
// If set, need to be passed as options to the handler constructor instead.
const {cacheName, networkTimeoutSeconds} = options;
delete options.cacheName;
delete options.networkTimeoutSeconds;

const pluginsMapping = {
backgroundSync: 'workbox.backgroundSync.Plugin',
broadcastCacheUpdate: 'workbox.broadcastCacheUpdate.Plugin',
cache: 'workbox.expiration.Plugin',
cacheExpiration: 'workbox.expiration.Plugin',
expiration: 'workbox.expiration.Plugin',
cacheableResponse: 'workbox.cacheableResponse.Plugin',
};

Expand Down Expand Up @@ -87,8 +75,7 @@ function getOptionsString(options = {}) {
}
}

module.exports = (runtimeCaching) => {
runtimeCaching = runtimeCaching || [];
module.exports = (runtimeCaching = []) => {
return runtimeCaching.map((entry) => {
const method = entry.method || 'GET';

Expand All @@ -100,6 +87,13 @@ module.exports = (runtimeCaching) => {
throw new Error(errors['handler-is-required']);
}

// This validation logic is a bit too gnarly for joi, so it's manually
// implemented here.
if (entry.options && entry.options.networkTimeoutSeconds &&
entry.handler !== 'networkFirst') {
throw new Error(errors['invalid-network-timeout-seconds']);
}

// urlPattern might be either a string or a RegExp object.
// If it's a string, it needs to be quoted. If it's a RegExp, it should
// be used as-is.
Expand Down
8 changes: 4 additions & 4 deletions test/workbox-build/node/entry-points/generate-sw-string.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ describe(`[workbox-build] entry-points/generate-sw-string.js (End to End)`, func
const runtimeCachingOptions = {
cacheName: 'test-cache-name',
plugins: [{}, {}],
cacheExpiration: {
expiration: {
maxEntries: 1,
maxAgeSeconds: 1,
},
Expand Down Expand Up @@ -395,7 +395,7 @@ describe(`[workbox-build] entry-points/generate-sw-string.js (End to End)`, func
]),
}]],
cacheableResponsePlugin: [[runtimeCachingOptions.cacheableResponse]],
cacheExpirationPlugin: [[runtimeCachingOptions.cacheExpiration]],
cacheExpirationPlugin: [[runtimeCachingOptions.expiration]],
importScripts: [[...DEFAULT_IMPORT_SCRIPTS]],
suppressWarnings: [[]],
precacheAndRoute: [[[], {}]],
Expand All @@ -406,7 +406,7 @@ describe(`[workbox-build] entry-points/generate-sw-string.js (End to End)`, func
it(`should support setting individual 'options' each, for multiple 'runtimeCaching' entries`, async function() {
const firstRuntimeCachingOptions = {
cacheName: 'first-cache-name',
cacheExpiration: {
expiration: {
maxEntries: 1,
maxAgeSeconds: 1,
},
Expand Down Expand Up @@ -442,7 +442,7 @@ describe(`[workbox-build] entry-points/generate-sw-string.js (End to End)`, func
plugins: ['workbox.cacheableResponse.Plugin'],
}]],
cacheableResponsePlugin: [[secondRuntimeCachingOptions.cacheableResponse]],
cacheExpirationPlugin: [[firstRuntimeCachingOptions.cacheExpiration]],
cacheExpirationPlugin: [[firstRuntimeCachingOptions.expiration]],
importScripts: [[...DEFAULT_IMPORT_SCRIPTS]],
suppressWarnings: [[]],
precacheAndRoute: [[[], {}]],
Expand Down
80 changes: 78 additions & 2 deletions test/workbox-build/node/entry-points/generate-sw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const tempy = require('tempy');

const cdnUtils = require('../../../../packages/workbox-build/src/lib/cdn-utils');
const copyWorkboxLibraries = require('../../../../packages/workbox-build/src/lib/copy-workbox-libraries');
const errors = require('../../../../packages/workbox-build/src/lib/errors');
const generateSW = require('../../../../packages/workbox-build/src/entry-points/generate-sw');
const validateServiceWorkerRuntime = require('../../../../infra/testing/validator/service-worker-runtime');

Expand Down Expand Up @@ -551,7 +552,7 @@ describe(`[workbox-build] entry-points/generate-sw.js (End to End)`, function()
const swDest = tempy.file();
const firstRuntimeCachingOptions = {
cacheName: 'first-cache-name',
cacheExpiration: {
expiration: {
maxEntries: 1,
maxAgeSeconds: 1,
},
Expand Down Expand Up @@ -592,7 +593,7 @@ describe(`[workbox-build] entry-points/generate-sw.js (End to End)`, function()
plugins: ['workbox.cacheableResponse.Plugin'],
}]],
cacheableResponsePlugin: [[secondRuntimeCachingOptions.cacheableResponse]],
cacheExpirationPlugin: [[firstRuntimeCachingOptions.cacheExpiration]],
cacheExpirationPlugin: [[firstRuntimeCachingOptions.expiration]],
importScripts: [[WORKBOX_SW_CDN_URL]],
suppressWarnings: [[]],
precacheAndRoute: [[[{
Expand Down Expand Up @@ -620,5 +621,80 @@ describe(`[workbox-build] entry-points/generate-sw.js (End to End)`, function()
],
}});
});

it(`should reject with a ValidationError when 'networkTimeoutSeconds' is used and handler is not 'networkFirst'`, async function() {
const swDest = tempy.file();
const runtimeCachingOptions = {
networkTimeoutSeconds: 1,
};
const runtimeCaching = [{
urlPattern: REGEXP_URL_PATTERN,
handler: 'networkOnly',
options: runtimeCachingOptions,
}];
const options = Object.assign({}, BASE_OPTIONS, {
runtimeCaching,
swDest,
});

try {
await generateSW(options);
throw new Error('Unexpected success.');
} catch (error) {
expect(error.message).to.include(errors['invalid-network-timeout-seconds']);
}
});

it(`should support 'networkTimeoutSeconds' when handler is 'networkFirst'`, async function() {
const swDest = tempy.file();
const networkTimeoutSeconds = 1;
const handler = 'networkFirst';

const runtimeCachingOptions = {
networkTimeoutSeconds,
plugins: [],
};
const runtimeCaching = [{
urlPattern: REGEXP_URL_PATTERN,
handler,
options: runtimeCachingOptions,
}];
const options = Object.assign({}, BASE_OPTIONS, {
runtimeCaching,
swDest,
});

const {count, size} = await generateSW(options);

expect(count).to.eql(6);
expect(size).to.eql(2421);
await validateServiceWorkerRuntime({swFile: swDest, expectedMethodCalls: {
[handler]: [[runtimeCachingOptions]],
importScripts: [[WORKBOX_SW_CDN_URL]],
suppressWarnings: [[]],
precacheAndRoute: [[[{
url: 'index.html',
revision: '3883c45b119c9d7e9ad75a1b4a4672ac',
}, {
url: 'page-1.html',
revision: '544658ab25ee8762dc241e8b1c5ed96d',
}, {
url: 'page-2.html',
revision: 'a3a71ce0b9b43c459cf58bd37e911b74',
}, {
url: 'styles/stylesheet-1.css',
revision: '934823cbc67ccf0d67aa2a2eeb798f12',
}, {
url: 'styles/stylesheet-2.css',
revision: '884f6853a4fc655e4c2dc0c0f27a227c',
}, {
url: 'webpackEntry.js',
revision: 'd41d8cd98f00b204e9800998ecf8427e',
}], {}]],
registerRoute: [
[REGEXP_URL_PATTERN, handler, DEFAULT_METHOD],
],
}});
});
});
});
21 changes: 9 additions & 12 deletions test/workbox-build/node/lib/runtime-caching-converter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,12 @@ function validate(runtimeCachingOptions, convertedOptions) {
.to.eql(strategiesOptions.networkTimeoutSeconds);
}

if (options.cache) {
if (options.cache.name) {
expect(strategiesOptions.cacheName).to.eql(options.cache.name);
delete options.cache.name;
}

if (Object.keys(options.cache).length > 0) {
expect(globalScope.workbox.expiration.Plugin.calledWith(options.cache)).to.be.true;
}
if (options.cacheName) {
expect(options.cacheName).to.eql(strategiesOptions.cacheName);
}

if (Object.keys(options.expiration).length > 0) {
expect(globalScope.workbox.expiration.Plugin.calledWith(options.expiration)).to.be.true;
}

if (options.cacheableResponse) {
Expand Down Expand Up @@ -127,8 +124,8 @@ describe(`[workbox-build] src/lib/utils/runtime-caching-converter.js`, function(
handler: 'networkFirst',
options: {
networkTimeoutSeconds: 20,
cache: {
name: 'abc-cache',
cacheName: 'abc-cache',
expiration: {
maxEntries: 5,
maxAgeSeconds: 50,
},
Expand All @@ -137,7 +134,7 @@ describe(`[workbox-build] src/lib/utils/runtime-caching-converter.js`, function(
urlPattern: '/test',
handler: 'staleWhileRevalidate',
options: {
cache: {
expiration: {
maxEntries: 10,
},
cacheableResponse: {
Expand Down