From f596930c8cfc5f855bfe5e446fc065bda0bf3153 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 24 May 2018 15:06:53 +0300 Subject: [PATCH] fix(docs-infra): ignore server-side redirected URLs in the SW (#19795) This allows URLs to be passed through to the server (where they are properly redirected), instead of serving `index.html` from the SW. Known issue: `/docs/` will be passed through to the server. `/docs` (without the trailing slash) will be correctly treated as a navigation URL and handled by the SW. We don't link to `/docs/` from within the app, but if there are external links to `/docs/` they will require a round-trip to the server and will not work in offline mode. PR Close #19795 --- aio/ngsw-config.json | 56 +++++++++++++++++++ aio/tests/deployment/shared/helpers.ts | 41 +++++++------- .../unit/testServiceWorkerRoutes.spec.ts | 48 ++++++++-------- 3 files changed, 98 insertions(+), 47 deletions(-) diff --git a/aio/ngsw-config.json b/aio/ngsw-config.json index 1341f188c0..f005b6b4b9 100644 --- a/aio/ngsw-config.json +++ b/aio/ngsw-config.json @@ -68,5 +68,61 @@ ] } } + ], + "navigationUrls": [ + "/**", + "!/**/*.*", + "!/**/*__*", + "!/**/*__*/**", + "!/**/stackblitz", + "!/**/stackblitz.html", + "!/api/*/**/*-*", + "!/api/**/AnimationStateDeclarationMetadata*", + "!/api/**/CORE_DIRECTIVES*", + "!/api/**/DirectiveMetadata*", + "!/api/**/HTTP_PROVIDERS*", + "!/api/**/NgFor", + "!/api/**/NgFor-*", + "!/api/**/PLATFORM_PIPES*", + "!/api/animate/**", + "!/api/api/**", + "!/api/common/Control*", + "!/api/common/MaxLengthValidator*", + "!/api/common/NgModel*", + "!/api/platform-browser/AnimationDriver*", + "!/api/testing/**", + "!/docs/?*", + "!/docs/*/**", + "!/guide/cli-quickstart", + "!/guide/cli-quickstart.html", + "!/guide/cli-quickstart/", + "!/guide/learning-angular", + "!/guide/learning-angular.html", + "!/guide/learning-angular/", + "!/guide/metadata", + "!/guide/metadata.html", + "!/guide/metadata/", + "!/guide/ngmodule", + "!/guide/ngmodule.html", + "!/guide/ngmodule/", + "!/guide/service-worker-getstart", + "!/guide/service-worker-getstart.html", + "!/guide/service-worker-getstart/", + "!/guide/service-worker-comm", + "!/guide/service-worker-comm.html", + "!/guide/service-worker-comm/", + "!/guide/service-worker-configref", + "!/guide/service-worker-configref.html", + "!/guide/service-worker-configref/", + "!/guide/webpack", + "!/guide/webpack.html", + "!/guide/webpack/", + "!/news", + "!/news.html", + "!/news/", + "!/styleguide", + "!/styleguide/**", + "!/testing", + "!/testing/**" ] } diff --git a/aio/tests/deployment/shared/helpers.ts b/aio/tests/deployment/shared/helpers.ts index 3a7f321a61..9b6a5d89f2 100644 --- a/aio/tests/deployment/shared/helpers.ts +++ b/aio/tests/deployment/shared/helpers.ts @@ -1,18 +1,35 @@ -import { resolve } from 'canonical-path'; +import { resolve as resolvePath } from 'canonical-path'; import { load as loadJson } from 'cjson'; import { readFileSync } from 'fs'; import { get as httpGet } from 'http'; import { get as httpsGet } from 'https'; +import { processNavigationUrls } from '../../../../packages/service-worker/config/src/generator'; import { FirebaseRedirector, FirebaseRedirectConfig } from '../../../tools/firebase-test-utils/FirebaseRedirector'; -const AIO_DIR = resolve(__dirname, '../../..'); +const AIO_DIR = resolvePath(__dirname, '../../..'); export function getRedirector() { return new FirebaseRedirector(loadRedirects()); } +export function getSwNavigationUrlChecker() { + const config = loadJson(`${AIO_DIR}/ngsw-config.json`); + const navigationUrlSpecs = processNavigationUrls('', config.navigationUrls); + + const includePatterns = navigationUrlSpecs + .filter(spec => spec.positive) + .map(spec => new RegExp(spec.regex)); + const excludePatterns = navigationUrlSpecs + .filter(spec => !spec.positive) + .map(spec => new RegExp(spec.regex)); + + return (url: string) => + includePatterns.some(regex => regex.test(url)) + && !excludePatterns.some(regex => regex.test(url)); +} + export function loadRedirects(): FirebaseRedirectConfig[] { const pathToFirebaseJSON = `${AIO_DIR}/firebase.json`; const contents = loadJson(pathToFirebaseJSON); @@ -46,26 +63,6 @@ export async function loadRemoteSitemapUrls(host: string) { return extractSitemapUrls(xml); } -export function loadSWRoutes() { - const pathToSWManifest = `${AIO_DIR}/ngsw-manifest.json`; - const contents = loadJson(pathToSWManifest); - const routes = contents.routing.routes; - return Object.keys(routes).map(route => { - const routeConfig = routes[route]; - switch (routeConfig.match) { - case 'exact': - return (url) => url === route; - case 'prefix': - return (url) => url.startsWith(route); - case 'regex': - const regex = new RegExp(route); - return (url) => regex.test(url); - default: - throw new Error(`unknown route config: ${route} - ${routeConfig.match}`); - } - }); -} - // Private functions function extractSitemapUrls(xml: string) { // Currently, all sitemaps use `angular.io` as host in URLs (which is fine since we only use the diff --git a/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts b/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts index d7a7be25fb..1f061252b8 100644 --- a/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts +++ b/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts @@ -1,40 +1,38 @@ -import { loadLegacyUrls, loadLocalSitemapUrls, loadSWRoutes } from '../shared/helpers'; +import { getSwNavigationUrlChecker, loadLegacyUrls, loadLocalSitemapUrls } from '../shared/helpers'; -// NOTE: The new `@angular/service-worker` does not support configurable routes. -xdescribe('service-worker routes', () => { +describe('ServiceWorker navigation URLs', () => { + const isNavigationUrl = getSwNavigationUrlChecker(); loadLocalSitemapUrls().forEach(url => { - it('should process URLs in the Sitemap', () => { - const routes = loadSWRoutes(); - expect(routes.some(test => test(url))).toBeTruthy(url); + it('should treat URLs in the Sitemap as navigation URLs', () => { + expect(isNavigationUrl(url)).toBeTruthy(url); }); }); loadLegacyUrls().forEach(urlPair => { const url = urlPair[0]; - it('should ignore legacy URLs that will be redirected', () => { - const routes = loadSWRoutes(); - expect(routes.some(test => test(url))).toBeFalsy(url); + it('should treat legacy URLs that will be redirected as non-navigation URLs', () => { + expect(isNavigationUrl(url)).toBeFalsy(url); }); }); - it('should ignore stackblitz URLs', () => { - const routes = loadSWRoutes(); - - // Normal StackBlitz URLs. - expect(routes.some(test => test('/generated/live-examples/toh-pt6/stackblitz.html'))).toBeFalsy(); - expect(routes.some(test => test('/generated/live-examples/toh-pt6/stackblitz'))).toBeFalsy(); - - // Embedded StackBlitz URLs. - expect(routes.some(test => test('/generated/live-examples/toh-pt6/stackblitz.html?ctl=1'))).toBeFalsy(); - expect(routes.some(test => test('/generated/live-examples/toh-pt6/stackblitz?ctl=1'))).toBeFalsy(); + it('should treat StackBlitz URLs as non-navigation URLs', () => { + expect(isNavigationUrl('/generated/live-examples/toh-pt6/stackblitz.html')).toBeFalsy(); + expect(isNavigationUrl('/generated/live-examples/toh-pt6/stackblitz')).toBeFalsy(); }); - it('should ignore URLs to files with extensions', () => { - const routes = loadSWRoutes(); - expect(routes.some(test => test('/generated/zips/animations/animations.zip'))).toBeFalsy(); - expect(routes.some(test => test('/generated/images/guide/animations/animation_auto.gif'))).toBeFalsy(); - expect(routes.some(test => test('/generated/ie-polyfills.min.js'))).toBeFalsy(); - expect(routes.some(test => test('/generated/docs/guide/animations.json'))).toBeFalsy(); + it('should treat URLs to files with extensions as non-navigation URLs', () => { + expect(isNavigationUrl('/generated/zips/animations/animations.zip')).toBeFalsy(); + expect(isNavigationUrl('/generated/images/guide/animations/animation_auto.gif')).toBeFalsy(); + expect(isNavigationUrl('/generated/ie-polyfills.min.js')).toBeFalsy(); + expect(isNavigationUrl('/generated/docs/guide/animations.json')).toBeFalsy(); + }); + + it('should treat `/docs*` URLs correctly', () => { + const navigationUrls = ['/docs', '/docs/']; + const nonNavigationUrls = ['/docs/foo', '/docs/foo/', '/docs/foo/bar']; + + navigationUrls.forEach(url => expect(isNavigationUrl(url)).toBeTruthy(url)); + nonNavigationUrls.forEach(url => expect(isNavigationUrl(url)).toBeFalsy(url)); }); });