feat(aio): Convert AIO to use the new Service Worker 5.0.0. #19795

alxhub · 2017-10-18T21:39:51Z

AIO is currently using a beta version of @angular/service-worker.
Since that was implemented, the SW has been rewritten and released
as part of Angular 5.0.0. This commit updates AIO to use the latest
implementation, with an appropriate configuration file that caches
the various AIO assets in useful ways.

gkalpak

🎉 ✨ (Quite excited about the dynamic content caching 😄)
A couple of comments/questions:

You can now remove the --ignore-packages @angular/service-worker part from here.
The SwUpdatesService spec needs to be updated too 😁
Do we still need ngsw-manifest.json?

gkalpak · 2017-10-18T21:59:27Z

aio/src/app/sw-updates/sw-updates.service.ts

@@ -31,38 +30,27 @@ export class SwUpdatesService implements OnDestroy {
  private checkInterval = 1000 * 60 * 60 * 6;   // 6 hours
  private onDestroy = new Subject();
  private checkForUpdateSubj = new Subject();
-  updateActivated = this.sw.updates
+  updateActivated = this.sw.activated
      .takeUntil(this.onDestroy)
      .do(evt => this.log(`Update event: ${JSON.stringify(evt)}`))


Nit: Update event --> Activation event

gkalpak · 2017-10-18T22:06:38Z

aio/src/main.ts

@@ -8,11 +8,4 @@ if (environment.production) {
  enableProdMode();
 }

-platformBrowserDynamic().bootstrapModule(AppModule).then(ref => {
-  if (environment.production && 'serviceWorker' in (navigator as any)) {


Will the ServiceWorker still be registered on production only?
(How does it know which environment it is in?)

The module is only added if environment.production is set.

Will that work fine if the SW support doesn't exist at runtime (e.g. ios)?

Yes, the module itself does feature detection to decide whether to register the SW.

gkalpak · 2017-10-18T22:12:20Z

aio/src/ngsw.json

+      ],
+      "urls": [
+        "https://fonts.googleapis.com/**",
+        "http://fonts.gstatic.com/s/**",


Shouldn't this be https too?

gkalpak · 2017-10-18T22:19:14Z

aio/src/ngsw.json

@@ -0,0 +1,50 @@
+{
+  "index": "/index.html",


How does the new ServiceWorker handle routing? (For reference this was our previous config.)

gkalpak · 2017-10-18T22:22:47Z

aio/src/ngsw.json

+        "/generated/docs/api/api-list.json"
+      ]
+    }
+  }]


AFAICT, it is possible to follow the previous config more closely (if not exactly) wrt to what gets cached and when.
Have you intentionally deviated from it?

lacolaco · 2017-10-19T01:26:17Z

aio/package.json

-    "sw-manifest": "ngu-sw-manifest --dist dist --in ngsw-manifest.json --out dist/ngsw-manifest.json",
-    "sw-copy": "cp node_modules/@angular/service-worker/bundles/worker-basic.min.js dist/",
+    "sw-manifest": "ngsw-config dist src/ngsw.json",
+    "sw-copy": "cp node_modules/@angular/service-worker/ngsw-worker.js dist/",


Just out of curiosity, couldn't you use assets of the CLI?

The SW script has to be registered at the root of the page's origin, it can't come from a subdirectory.

webmaxru · 2017-10-26T05:27:30Z

aio/src/ngsw.json

+  "index": "/index.html",
+  "assetGroups": [{
+    "name": "site",
+    "mode": "prefetch",


Do you mean installMode?

webmaxru · 2017-10-26T05:29:14Z

aio/src/ngsw.json

+        "/pwa-manifest.json",
+        "/app/search/search-worker.js"
+      ],
+      "urls": [


I'd move this block with fonts to a separate group, because it will not follow prefetch rule anyway

It's conceptually part of the site.

webmaxru · 2017-10-26T08:47:02Z

aio/src/app/sw-updates/sw-updates.service.ts

@@ -31,38 +30,27 @@ export class SwUpdatesService implements OnDestroy {
  private checkInterval = 1000 * 60 * 60 * 6;   // 6 hours
  private onDestroy = new Subject();
  private checkForUpdateSubj = new Subject();
-  updateActivated = this.sw.updates
+  updateActivated = this.sw.activated
      .takeUntil(this.onDestroy)
      .do(evt => this.log(`Update event: ${JSON.stringify(evt)}`))


My IDE says we have to add import 'rxjs/add/operator/do';

Splaktar

This will be a really useful 'real world' example once it gets to production!

Splaktar · 2017-11-06T21:19:27Z

aio/package.json

@@ -77,7 +77,7 @@
    "@angular/platform-browser-dynamic": "^5.0.0-beta.3",
    "@angular/platform-server": "^5.0.0-beta.3",
    "@angular/router": "^5.0.0-beta.3",
-    "@angular/service-worker": "^1.0.0-beta.16",
+    "@angular/service-worker": "5.0.0-rc.2",


Need to rebase to master to pick up 6b74883 and then update this to 5.0.0. It probably also worth using the Angular CLI 1.6.0-beta.0 release that supports Angular Service Worker.

IgorMinar · 2017-11-22T00:25:46Z

@alxhub can you please resurrect this PR and get it into a mergable state? thanks!

alxhub · 2017-11-29T21:48:54Z

@IgorMinar PTAL.

IgorMinar

I'm a bit concerned about the impact on ergonomics, let's discuss how we can resolve that - I'm afraid that this will require an API change/addition unless you have other ideas. Otherwise this looks good after the nitpicks are resolved.

IgorMinar · 2017-11-30T18:54:27Z

aio/package.json

-    "sw-manifest": "ngu-sw-manifest --dist dist --in ngsw-manifest.json --out dist/ngsw-manifest.json",
-    "sw-copy": "cp node_modules/@angular/service-worker/bundles/worker-basic.min.js dist/",
+    "sw-manifest": "ngsw-config dist src/ngsw-config.json",
+    "sw-copy": "cp node_modules/@angular/service-worker/ngsw-worker.js dist/",


can't you update to cli@next and use the new build integration?

Yes, I could. But I don't want to make more changes to AIO's build system than I need to.

FWIW, we should soon upgrade to the latest-ish cli in #18428. So, maybe after that is merged you can use the build integration.

FWIW, we are now on cli@1.6.3.

IgorMinar · 2017-11-30T18:55:20Z

aio/scripts/_payload-limits.json

@@ -1,3 +1,3 @@
 {
-"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37068}}}
+"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37070}}}


odd.. why the change to polyfills size?

No idea - it was failing.

It couldn't be failing for a 2B diff 😕
(FWIW, I think the polyfills size has been 37070 for a while, but it wasn't updated in the limits, since it is a small changed - much lower than 1%).

IgorMinar · 2017-11-30T18:59:41Z

aio/src/index.html

@@ -2,7 +2,7 @@
 <html>
 <head>
  <meta charset="utf-8">
-  <title>Angular Docs</title>
+  <title>Angular Docs!!!</title>


It's not fixed. Did you forget to push the change?

IgorMinar · 2017-11-30T19:19:09Z

aio/scripts/_payload-limits.json

@@ -1,3 +1,3 @@
 {
-"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37068}}}
+"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37070}}}


Also, I'd expect the main size to decrease. Isn't that the case or is it less than 1% so the size check in CI doesn't fail?

I'm not sure what you mean. It's probably less than 1%, this doesn't change much code.

IgorMinar · 2017-11-30T19:20:39Z

aio/src/app/app.module.ts

@@ -89,7 +92,8 @@ export const svgIconProviders = [
    MatTabsModule,
    MatToolbarModule,
    SwUpdatesModule,
-    SharedModule
+    SharedModule,
+    environment.production ? ServiceWorkerModule.register('/ngsw-worker.js') : [],


this is a bit odd, because it means that now you have to conditionally or optionally inject the ngsw services which makes the application code not very ergonomic.

Good point. I will fix that.

IgorMinar · 2017-11-30T19:22:38Z

aio/src/app/sw-updates/sw-updates.service.ts

+      this.updateActivated = Observable.never<string>().takeUntil(this.onDestroy);
+      return;
+    }
+    this.sw = injector.get(SwUpdate);


ouch. that's ugly. can we do better? either inject it optionally, or even better always register the providers and inject them, but if we are on browser without SW support or in Dev mode, then make the services inert.. or something like that. We should really rethink this, because the current ergonomics are bad. I believe that this is what Stephen was pointing out as well...

IgorMinar · 2017-11-30T19:23:53Z

aio/src/main.ts

@@ -8,11 +8,4 @@ if (environment.production) {
  enableProdMode();
 }

-platformBrowserDynamic().bootstrapModule(AppModule).then(ref => {
-  if (environment.production && 'serviceWorker' in (navigator as any)) {


Will that work fine if the SW support doesn't exist at runtime (e.g. ios)?

IgorMinar · 2017-11-30T19:24:35Z

aio/src/ngsw-config.json

+{
+  "index": "/index.html",
+  "assetGroups": [{
+    "name": "site",


please rename this to "app-shell"

IgorMinar · 2017-11-30T19:29:24Z

aio/src/ngsw-config.json

+      ]
+    }
+  }, {
+    "name": "docs",


there are actually two docs content groups - the ones that are available offline, and those that are lazy downloaded.

The offline docs include all marketing pages and images under generated/marketing/ I believe - check what the current site is fetching: https://angular.io/ngsw-manifest.json

The the remaining docs and assets should be downloaded on as needed basis. Your proposal changes the update behavior (compared to current angular.io) - I'm fine experimenting with the prefetch mode. We'll see how well this user experience works.

As discussed, the docs group has been moved to prefetch.

gkalpak · 2017-11-30T23:25:38Z

aio/scripts/_payload-limits.json

@@ -1,3 +1,3 @@
 {
-"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37068}}}
+"aio":{"master":{"change":"application","gzip7":{"inline":925,"main":119519,"polyfills":11863},"gzip9":{"inline":925,"main":119301,"polyfills":11861},"uncompressed":{"inline":1533,"main":486493,"polyfills":37070}}}


It couldn't be failing for a 2B diff 😕
(FWIW, I think the polyfills size has been 37070 for a while, but it wasn't updated in the limits, since it is a small changed - much lower than 1%).

gkalpak · 2017-11-30T23:27:12Z

aio/src/app/app.module.ts

@@ -53,6 +54,8 @@ import { WindowToken, windowProvider } from 'app/shared/window';

 import { SharedModule } from 'app/shared/shared.module';

+import {environment} from '../environments/environment';


Inconsistent spacing inside {...}.
_{I don't like the whitespace either, but... ¯\_(ツ)_/¯}

Did you mean "Not fixed"? 😁

gkalpak · 2017-11-30T23:32:11Z

aio/src/main.ts

-platformBrowserDynamic().bootstrapModule(AppModule).then(ref => {
-  if (environment.production && 'serviceWorker' in (navigator as any)) {
-    const appRef: ApplicationRef = ref.injector.get(ApplicationRef);
-    appRef.isStable.first().subscribe(() => {


IIRC, we added this to improve TTI or something.
Is this now handled by the ServiceWorkerModule internally?

PR Close #19795

…vkit/build-angular to 0.7.0-rc.3 (#19795) PR Close #19795

…#19795) PR Close #19795

PR Close #19795

…19795) The is a bug in versions 0.6.8+ that breaks when trying to use Jasmine's mock clock. Related to angular/angular-cli#11626. PR Close #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

`%%DEPLOYMENT_HOST%%` has been assumed to be the host prefix for sitemap URLs since bf29936, but afaict this was never the case. PR Close angular#19795

`%%DEPLOYMENT_HOST%%` has been assumed to be the host prefix for sitemap URLs since bf29936, but afaict this was never the case. PR Close #19795 PR Close #25820

`%%DEPLOYMENT_HOST%%` has been assumed to be the host prefix for sitemap URLs since bf29936, but afaict this was never the case. PR Close angular#19795

…ngular#19795) AIO is currently using a beta version of @angular/service-worker. Since that was implemented, the SW has been rewritten and released as part of Angular 5.0.0. This commit updates AIO to use the latest implementation, with an appropriate configuration file that caches the various AIO assets in useful ways. PR Close angular#19795

PR Close angular#19795

…angular#19795) PR Close angular#19795

…gular#19795) PR Close angular#19795

PR Close angular#19795

) PR Close angular#19795

PR Close angular#19795

…ular#19795) PR Close angular#19795

PR Close angular#19795

…vkit/build-angular to 0.7.0-rc.3 (angular#19795) PR Close angular#19795

…angular#19795) PR Close angular#19795

PR Close angular#19795

…ngular#19795) The is a bug in versions 0.6.8+ that breaks when trying to use Jasmine's mock clock. Related to angular/angular-cli#11626. PR Close angular#19795

…r#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 angular#19795

angular-automatic-lock-bot · 2019-09-13T23:53:05Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Oct 18, 2017

gkalpak reviewed Oct 18, 2017

View reviewed changes

gkalpak added comp: aio state: WIP target: major This PR is targeted for the next major release labels Oct 18, 2017

lacolaco reviewed Oct 19, 2017

View reviewed changes

webmaxru reviewed Oct 26, 2017

View reviewed changes

lacolaco mentioned this pull request Oct 26, 2017

bug(service-worker): / path is omitted by ngsw-config #19945

Closed

IgorMinar added target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release labels Nov 3, 2017

Splaktar reviewed Nov 6, 2017

View reviewed changes

IgorMinar added this to the 5.0.x milestone Nov 22, 2017

IgorMinar added the feature Issue that requests a new feature label Nov 22, 2017

alxhub force-pushed the aio-new-sw branch 2 times, most recently from fb0cbb6 to 35707ab Compare November 29, 2017 16:16

alxhub added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Nov 29, 2017

alxhub force-pushed the aio-new-sw branch from 35707ab to 571a600 Compare November 29, 2017 16:50

alxhub requested a review from IgorMinar November 29, 2017 21:48

alxhub force-pushed the aio-new-sw branch from 571a600 to 3eb0266 Compare November 30, 2017 18:43

IgorMinar previously requested changes Nov 30, 2017

View reviewed changes

gkalpak reviewed Nov 30, 2017

View reviewed changes

IgorMinar removed this from the 5.0.x milestone Dec 1, 2017

alxhub force-pushed the aio-new-sw branch 3 times, most recently from 7e7d935 to 9d4d9d4 Compare December 5, 2017 17:27

matsko pushed a commit that referenced this pull request Aug 27, 2018

build(docs-infra): update size limits (#19795)

c73ebe7

PR Close #19795

matsko pushed a commit that referenced this pull request Aug 27, 2018

build(docs-infra): upgrade @angular/cli to 6.1.0-rc.3 and @angular-de…

2569fd4

…vkit/build-angular to 0.7.0-rc.3 (#19795) PR Close #19795

matsko pushed a commit that referenced this pull request Aug 27, 2018

build(docs-infra): upgrade @angular/* to 6.1.0-rc.3 and rxjs to 6.2.2 (…

4d83a0a

…#19795) PR Close #19795

matsko pushed a commit that referenced this pull request Aug 27, 2018

build(docs-infra): update size limits (#19795)

864c22f

PR Close #19795

gkalpak mentioned this pull request Sep 5, 2018

[6.1.x] test(docs-infra): fix double-slash in URL of aio_monitoring test #25820

Closed

gkalpak mentioned this pull request Sep 18, 2018

[6.1.x] update angular.io to the new ServiceWorker #25997

Closed

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

refactor(docs-infra): remove unused images (angular#19795)

2fffc05

PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

refactor(docs-infra): move concept icons to more appropriate location (…

99f0ff7

…angular#19795) PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

fix(docs-infra): correctly show icon for fetch error when offline (an…

fe6592c

…gular#19795) PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): enable ServiceWorker in cli config (angular#19795)

3eda251

PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): remove the dependency on rxjs-compat (angular#19795

dade06d

) PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): remove unused dependencies (angular#19795)

920b5a6

PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): upgrade @angular/cli to 6.0.3 (angular#19795)

b3d431f

PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): upgrade @angular/* to 6.0.2 and rxjs to 6.1.0 (ang…

1ee5908

…ular#19795) PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): upgrade @angular/material to 6.0.2 (angular#19795)

17ac2e9

PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): update size limits (angular#19795)

2e33c40

PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): upgrade @angular/cli to 6.1.0-rc.3 and @angular-de…

a640a07

…vkit/build-angular to 0.7.0-rc.3 (angular#19795) PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): upgrade @angular/* to 6.1.0-rc.3 and rxjs to 6.2.2 (…

54d0ad3

…angular#19795) PR Close angular#19795

FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019

build(docs-infra): update size limits (angular#19795)

f3d1066

PR Close angular#19795

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

		@@ -53,6 +54,8 @@ import { WindowToken, windowProvider } from 'app/shared/window';

		import { SharedModule } from 'app/shared/shared.module';

		import {environment} from '../environments/environment';

feat(aio): Convert AIO to use the new Service Worker 5.0.0. #19795

feat(aio): Convert AIO to use the new Service Worker 5.0.0. #19795

Conversation

alxhub commented Oct 18, 2017

gkalpak left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Splaktar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IgorMinar commented Nov 22, 2017

alxhub commented Nov 29, 2017

IgorMinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alxhub Nov 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 13, 2019

gkalpak left a comment •

edited

Loading

alxhub Nov 30, 2017 •

edited

Loading