fix(aio): wait for the app to stabilize before registering the SW (#2…

…2483)

This commit also waits for the app to stabilize, before starting to
check for ServiceWorker updates. This avoids setting up a long timeout,
which would prevent the app from stabilizing and thus cause issues with
Protractor.

PR Close #22483

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

@@ -1,4 +1,4 @@
-import { ReflectiveInjector } from '@angular/core';
+import { ApplicationRef, ReflectiveInjector } from '@angular/core';
 import { fakeAsync, tick } from '@angular/core/testing';
 import { NgServiceWorker } from '@angular/service-worker';
 import { Subject } from 'rxjs';
@@ -9,23 +9,26 @@ import { SwUpdatesService } from './sw-updates.service';
 
 describe('SwUpdatesService', () => {
   let injector: ReflectiveInjector;
+  let appRef: MockApplicationRef;
   let service: SwUpdatesService;
   let sw: MockNgServiceWorker;
   let checkInterval: number;
 
   // Helpers
   // NOTE:
-  //   Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated
-  //   inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't run
-  //   `setup()` in a `beforeEach()` block. We use the `run()` helper to call `setup()` inside each
-  //   test's zone.
+  //   Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated and
+  //   destroyed inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't
+  //   run `setup()`/`tearDown()` in `beforeEach()`/`afterEach()` blocks. We use the `run()` helper
+  //   to call them inside each test's zone.
   const setup = () => {
     injector = ReflectiveInjector.resolveAndCreate([
+      { provide: ApplicationRef, useClass: MockApplicationRef },
       { provide: Logger, useClass: MockLogger },
       { provide: NgServiceWorker, useClass: MockNgServiceWorker },
       SwUpdatesService
     ]);
 
+    appRef = injector.get(ApplicationRef);
     service = injector.get(SwUpdatesService);
     sw = injector.get(NgServiceWorker);
     checkInterval = (service as any).checkInterval;
@@ -42,11 +45,18 @@ describe('SwUpdatesService', () => {
     expect(service).toBeTruthy();
   }));
 
-  it('should immediately check for updates when instantiated', run(() => {
+  it('should start checking for updates when instantiated (once the app stabilizes)', run(() => {
+    expect(sw.checkForUpdate).not.toHaveBeenCalled();
+
+    appRef.isStable.next(false);
+    expect(sw.checkForUpdate).not.toHaveBeenCalled();
+
+    appRef.isStable.next(true);
     expect(sw.checkForUpdate).toHaveBeenCalled();
   }));
 
   it('should schedule a new check if there is no update available', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.checkForUpdate.calls.reset();
 
     sw.$$checkForUpdateSubj.next(false);
@@ -58,6 +68,7 @@ describe('SwUpdatesService', () => {
   })));
 
   it('should activate new updates immediately', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.checkForUpdate.calls.reset();
 
     sw.$$checkForUpdateSubj.next(true);
@@ -69,13 +80,15 @@ describe('SwUpdatesService', () => {
   })));
 
   it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.$$checkForUpdateSubj.next(true);
     tick(checkInterval);
 
     expect(sw.activateUpdate).toHaveBeenCalledWith(null);
   })));
 
   it('should schedule a new check after activating the update', fakeAsync(run(() => {
+    appRef.isStable.next(true);
     sw.checkForUpdate.calls.reset();
     sw.$$checkForUpdateSubj.next(true);
 
@@ -103,6 +116,7 @@ describe('SwUpdatesService', () => {
 
   describe('when destroyed', () => {
     it('should not schedule a new check for update (after current check)', fakeAsync(run(() => {
+      appRef.isStable.next(true);
       sw.checkForUpdate.calls.reset();
 
       service.ngOnDestroy();
@@ -113,6 +127,7 @@ describe('SwUpdatesService', () => {
     })));
 
     it('should not schedule a new check for update (after activating an update)', fakeAsync(run(() => {
+      appRef.isStable.next(true);
       sw.checkForUpdate.calls.reset();
 
       sw.$$checkForUpdateSubj.next(true);
@@ -141,6 +156,10 @@ describe('SwUpdatesService', () => {
 });
 
 // Mocks
+class MockApplicationRef {
+  isStable = new Subject<boolean>();
+}
+
 class MockLogger {
   log = jasmine.createSpy('MockLogger.log');
 }

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

@@ -1,7 +1,8 @@
-import { Injectable, OnDestroy } from '@angular/core';
+import { ApplicationRef, Injectable, OnDestroy } from '@angular/core';
 import { NgServiceWorker } from '@angular/service-worker';
-import { concat, of, Subject } from 'rxjs';
-import { debounceTime, filter, map, startWith, take, takeUntil, tap } from 'rxjs/operators';
+import { concat, Subject } from 'rxjs';
+import { debounceTime, defaultIfEmpty, filter, first, map, startWith, takeUntil, tap } from 'rxjs/operators';
+
 import { Logger } from 'app/shared/logger.service';
 
 
@@ -29,13 +30,12 @@ export class SwUpdatesService implements OnDestroy {
       map(({version}) => version),
   );
 
-  constructor(private logger: Logger, private sw: NgServiceWorker) {
-    this.checkForUpdateSubj
-        .pipe(
-            debounceTime(this.checkInterval),
-            startWith<void>(undefined),
-            takeUntil(this.onDestroy),
-        )
+  constructor(appRef: ApplicationRef, private logger: Logger, private sw: NgServiceWorker) {
+    const appIsStable$ = appRef.isStable.pipe(first(v => v));
+    const checkForUpdates$ = this.checkForUpdateSubj.pipe(debounceTime(this.checkInterval), startWith<void>(undefined));
+
+    concat(appIsStable$, checkForUpdates$)
+        .pipe(takeUntil(this.onDestroy))
         .subscribe(() => this.checkForUpdate());
   }
 
@@ -51,11 +51,12 @@ export class SwUpdatesService implements OnDestroy {
 
   private checkForUpdate() {
     this.log('Checking for update...');
-    // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137.
-    // TODO (gkalpak): Remove once #137 is fixed.
-    concat(this.sw.checkForUpdate(), of(false))
+    this.sw.checkForUpdate()
         .pipe(
-            take(1),
+            // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137.
+            // TODO (gkalpak): Remove once #137 is fixed.
+            defaultIfEmpty(false),
+            first(),
             tap(v => this.log(`Update available: ${v}`)),
         )
         .subscribe(v => v ? this.activateUpdate() : this.scheduleCheckForUpdate());

aio/src/main.ts

@@ -12,7 +12,7 @@ if (environment.production) {
 platformBrowserDynamic().bootstrapModule(AppModule).then(ref => {
   if (environment.production && 'serviceWorker' in (navigator as any)) {
     const appRef: ApplicationRef = ref.injector.get(ApplicationRef);
-    appRef.isStable.pipe(first()).subscribe(() => {
+    appRef.isStable.pipe(first(v => v)).subscribe(() => {
       (navigator as any).serviceWorker.register('/worker-basic.min.js');
     });
   }