From a2068523fd284f5160fe7f4ce696fd3cd01f40f5 Mon Sep 17 00:00:00 2001 From: klemenoslaj Date: Mon, 24 Aug 2020 16:04:17 +0200 Subject: [PATCH] feat(service-worker): add the option to prefer network for navigation requests (#38565) This commit introduces a new option for the service worker, called `navigationRequestStrategy`, which adds the possibility to force the service worker to always create a network request for navigation requests. This enables the server redirects while retaining the offline behavior. Fixes #38194 PR Close #38565 --- aio/content/guide/service-worker-config.md | 35 ++++++++++++ .../service-worker/config/config.d.ts | 1 + packages/service-worker/config/schema.json | 8 +++ .../service-worker/config/src/generator.ts | 1 + packages/service-worker/config/src/in.ts | 1 + .../config/test/generator_spec.ts | 3 ++ .../service-worker/test/integration_spec.ts | 2 + .../service-worker/worker/src/app-version.ts | 12 +++++ .../service-worker/worker/src/manifest.ts | 1 + .../service-worker/worker/test/data_spec.ts | 1 + .../service-worker/worker/test/happy_spec.ts | 53 +++++++++++++++++++ .../service-worker/worker/testing/mock.ts | 1 + .../service-worker/worker/testing/scope.ts | 1 + 13 files changed, 120 insertions(+) diff --git a/aio/content/guide/service-worker-config.md b/aio/content/guide/service-worker-config.md index 389033844e..6135f2f218 100644 --- a/aio/content/guide/service-worker-config.md +++ b/aio/content/guide/service-worker-config.md @@ -267,6 +267,12 @@ By default, these criteria are: 1. The URL must not contain a file extension (i.e. a `.`) in the last path segment. 2. The URL must not contain `__`. +
+ +To configure whether navigation requests are sent through to the network or not, see the [navigationRequestStrategy](#navigation-request-strategy) section. + +
+ ### Matching navigation request URLs While these default criteria are fine in most cases, it is sometimes desirable to configure different rules. For example, you may want to ignore specific routes (that are not part of the Angular app) and pass them through to the server. @@ -285,3 +291,32 @@ If the field is omitted, it defaults to: '!/**/*__*/**', // Exclude URLs containing `__` in any other segment. ] ``` + +{@a navigation-request-strategy} + +## `navigationRequestStrategy` + +This optional property enables you to configure how the service worker handles navigation requests: + +```json +{ + "navigationRequestStrategy": "freshness" +} +``` + +Possible values: + +- `'performance'`: The default setting. Serves the specified [index file](#index-file), which is typically cached. +- `'freshness'`: Passes the requests through to the network and falls back to the `performance` behavior when offline. + This value is useful when the server redirects the navigation requests elsewhere using an HTTP redirect (3xx status code). + Reasons for using this value include: + - Redirecting to an authentication website when authentication is not handled by the application. + - Redirecting specific URLs to avoid breaking existing links/bookmarks after a website redesign. + - Redirecting to a different website, such as a server-status page, while a page is temporarily down. + +
+ +The `freshness` strategy usually results in more requests sent to the server, which can increase response latency. +It is recommended that you use the default performance strategy whenever possible. + +
\ No newline at end of file diff --git a/goldens/public-api/service-worker/config/config.d.ts b/goldens/public-api/service-worker/config/config.d.ts index 575115c56e..75a007d98b 100644 --- a/goldens/public-api/service-worker/config/config.d.ts +++ b/goldens/public-api/service-worker/config/config.d.ts @@ -14,6 +14,7 @@ export declare interface Config { assetGroups?: AssetGroup[]; dataGroups?: DataGroup[]; index: string; + navigationRequestStrategy?: 'freshness' | 'performance'; navigationUrls?: string[]; } diff --git a/packages/service-worker/config/schema.json b/packages/service-worker/config/schema.json index 5df950afab..a8cfecd218 100644 --- a/packages/service-worker/config/schema.json +++ b/packages/service-worker/config/schema.json @@ -161,6 +161,14 @@ "type": "string" }, "uniqueItems": true + }, + "navigationRequestStrategy": { + "enum": [ + "freshness", + "performance" + ], + "default": "performance", + "description": "The Angular service worker can use two request strategies for navigation requests. 'performance', the default, skips navigation requests. The other strategy, 'freshness', forces all navigation requests through the network." } }, "required": [ diff --git a/packages/service-worker/config/src/generator.ts b/packages/service-worker/config/src/generator.ts index 21127d3ade..61f3e0e5da 100644 --- a/packages/service-worker/config/src/generator.ts +++ b/packages/service-worker/config/src/generator.ts @@ -39,6 +39,7 @@ export class Generator { dataGroups: this.processDataGroups(config), hashTable: withOrderedKeys(unorderedHashTable), navigationUrls: processNavigationUrls(this.baseHref, config.navigationUrls), + navigationRequestStrategy: config.navigationRequestStrategy ?? 'performance', }; } diff --git a/packages/service-worker/config/src/in.ts b/packages/service-worker/config/src/in.ts index 35a913332e..09fe07563b 100644 --- a/packages/service-worker/config/src/in.ts +++ b/packages/service-worker/config/src/in.ts @@ -27,6 +27,7 @@ export interface Config { assetGroups?: AssetGroup[]; dataGroups?: DataGroup[]; navigationUrls?: string[]; + navigationRequestStrategy?: 'freshness'|'performance'; } /** diff --git a/packages/service-worker/config/test/generator_spec.ts b/packages/service-worker/config/test/generator_spec.ts index c2600490e5..f94a2125d3 100644 --- a/packages/service-worker/config/test/generator_spec.ts +++ b/packages/service-worker/config/test/generator_spec.ts @@ -117,6 +117,7 @@ describe('Generator', () => { {positive: true, regex: '^http:\\/\\/example\\.com\\/included$'}, {positive: false, regex: '^http:\\/\\/example\\.com\\/excluded$'}, ], + navigationRequestStrategy: 'performance', hashTable: { '/test/foo/test.html': '18f6f8eb7b1c23d2bb61bff028b83d867a9e4643', '/test/index.html': 'a54d88e06612d820bc3be72877c74f257b561b19', @@ -149,6 +150,7 @@ describe('Generator', () => { {positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*$'}, {positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*\\/.*$'}, ], + navigationRequestStrategy: 'performance', hashTable: {}, }); }); @@ -249,6 +251,7 @@ describe('Generator', () => { {positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*$'}, {positive: false, regex: '^\\/(?:.+\\/)?[^/]*__[^/]*\\/.*$'}, ], + navigationRequestStrategy: 'performance', hashTable: { '/index.html': 'a54d88e06612d820bc3be72877c74f257b561b19', '/main.js': '41347a66676cdc0516934c76d9d13010df420f2c', diff --git a/packages/service-worker/test/integration_spec.ts b/packages/service-worker/test/integration_spec.ts index 1eb7f01c0b..eeee1ad2b7 100644 --- a/packages/service-worker/test/integration_spec.ts +++ b/packages/service-worker/test/integration_spec.ts @@ -47,6 +47,7 @@ const manifest: Manifest = { cacheQueryOptions: {ignoreVary: true}, }], navigationUrls: [], + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(dist), }; @@ -64,6 +65,7 @@ const manifestUpdate: Manifest = { cacheQueryOptions: {ignoreVary: true}, }], navigationUrls: [], + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(distUpdate), }; diff --git a/packages/service-worker/worker/src/app-version.ts b/packages/service-worker/worker/src/app-version.ts index a4c0cc9823..7e1054d85c 100644 --- a/packages/service-worker/worker/src/app-version.ts +++ b/packages/service-worker/worker/src/app-version.ts @@ -185,6 +185,18 @@ export class AppVersion implements UpdateSource { // Next, check if this is a navigation request for a route. Detect circular // navigations by checking if the request URL is the same as the index URL. if (this.adapter.normalizeUrl(req.url) !== this.indexUrl && this.isNavigationRequest(req)) { + if (this.manifest.navigationRequestStrategy === 'freshness') { + // For navigation requests the freshness was configured. The request will always go trough + // the network and fallback to default `handleFetch` behavior in case of failure. + try { + return await this.scope.fetch(req); + } catch { + // Navigation request failed - application is likely offline. + // Proceed forward to the default `handleFetch` behavior, where + // `indexUrl` will be requested and it should be available in the cache. + } + } + // This was a navigation request. Re-enter `handleFetch` with a request for // the URL. return this.handleFetch(this.adapter.newRequest(this.indexUrl), context); diff --git a/packages/service-worker/worker/src/manifest.ts b/packages/service-worker/worker/src/manifest.ts index ec68a51780..cb9520b13e 100644 --- a/packages/service-worker/worker/src/manifest.ts +++ b/packages/service-worker/worker/src/manifest.ts @@ -18,6 +18,7 @@ export interface Manifest { assetGroups?: AssetGroupConfig[]; dataGroups?: DataGroupConfig[]; navigationUrls: {positive: boolean, regex: string}[]; + navigationRequestStrategy: 'freshness'|'performance'; hashTable: {[url: string]: string}; } diff --git a/packages/service-worker/worker/test/data_spec.ts b/packages/service-worker/worker/test/data_spec.ts index 05bbf4c21e..448b95861b 100644 --- a/packages/service-worker/worker/test/data_spec.ts +++ b/packages/service-worker/worker/test/data_spec.ts @@ -93,6 +93,7 @@ const manifest: Manifest = { }, ], navigationUrls: [], + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(dist), }; diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index e4cdaaa320..09c5a374d7 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -74,6 +74,7 @@ const brokenManifest: Manifest = { }], dataGroups: [], navigationUrls: processNavigationUrls(''), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(brokenFs, {'/foo.txt': true}), }; @@ -105,6 +106,7 @@ const brokenLazyManifest: Manifest = { ], dataGroups: [], navigationUrls: processNavigationUrls(''), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(brokenFs, {'/bar.txt': true}), }; @@ -198,6 +200,7 @@ const manifest: Manifest = { }, ], navigationUrls: processNavigationUrls(''), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(dist), }; @@ -256,6 +259,7 @@ const manifestUpdate: Manifest = { '!/ignored/file1', '!/ignored/dir/**', ]), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(distUpdate), }; @@ -983,6 +987,7 @@ describe('Driver', () => { }, ], navigationUrls: processNavigationUrls(baseHref), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(distDir, {}, baseHref), }); @@ -1343,6 +1348,7 @@ describe('Driver', () => { } ], navigationUrls: processNavigationUrls('./'), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(distDir, {}, './'), }); @@ -1754,6 +1760,7 @@ describe('Driver', () => { }], dataGroups: [], navigationUrls: processNavigationUrls(''), + navigationRequestStrategy: 'performance', hashTable: tmpHashTableForFs(fileSystem), }; @@ -1922,6 +1929,52 @@ describe('Driver', () => { }); }); }); + + describe('navigationRequestStrategy', () => { + it('doesn\'t create navigate request in performance mode', async () => { + await makeRequest(scope, '/foo.txt'); + await driver.initialized; + await server.clearRequests(); + + // Create multiple navigation requests to prove no navigation request was made. + // By default the navigation request is not sent, it's replaced + // with the index request - thus, the `this is foo` value. + expect(await makeNavigationRequest(scope, '/', '')).toBe('this is foo'); + expect(await makeNavigationRequest(scope, '/foo', '')).toBe('this is foo'); + expect(await makeNavigationRequest(scope, '/foo/bar', '')).toBe('this is foo'); + + server.assertNoOtherRequests(); + }); + + it('sends the request to the server in freshness mode', async () => { + const {server, scope, driver} = createSwForFreshnessStrategy(); + + await makeRequest(scope, '/foo.txt'); + await driver.initialized; + await server.clearRequests(); + + // Create multiple navigation requests to prove the navigation request is constantly made. + // When enabled, the navigation request is made each time and not replaced + // with the index request - thus, the `null` value. + expect(await makeNavigationRequest(scope, '/', '')).toBe(null); + expect(await makeNavigationRequest(scope, '/foo', '')).toBe(null); + expect(await makeNavigationRequest(scope, '/foo/bar', '')).toBe(null); + + server.assertSawRequestFor('/'); + server.assertSawRequestFor('/foo'); + server.assertSawRequestFor('/foo/bar'); + server.assertNoOtherRequests(); + }); + + function createSwForFreshnessStrategy() { + const freshnessManifest: Manifest = {...manifest, navigationRequestStrategy: 'freshness'}; + const server = serverBuilderBase.withManifest(freshnessManifest).build(); + const scope = new SwTestHarnessBuilder().withServerState(server).build(); + const driver = new Driver(scope, scope, new CacheDatabase(scope, scope)); + + return {server, scope, driver}; + } + }); }); })(); diff --git a/packages/service-worker/worker/testing/mock.ts b/packages/service-worker/worker/testing/mock.ts index 8b51e38ad4..62bb1c84e0 100644 --- a/packages/service-worker/worker/testing/mock.ts +++ b/packages/service-worker/worker/testing/mock.ts @@ -248,6 +248,7 @@ export function tmpManifestSingleAssetGroup(fs: MockFileSystem): Manifest { }, ], navigationUrls: [], + navigationRequestStrategy: 'performance', hashTable, }; } diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts index 44df03a2ed..57e77055ee 100644 --- a/packages/service-worker/worker/testing/scope.ts +++ b/packages/service-worker/worker/testing/scope.ts @@ -346,6 +346,7 @@ export class ConfigBuilder { index: '/index.html', assetGroups, navigationUrls: [], + navigationRequestStrategy: 'performance', hashTable, }; }