fix(aio): do not show new document until embedded components are ready (#18428)

Previously, the document was shown as soon as the HTML was received, but before
the embedded components were ready (e.g. downloaded and instantiated). This
caused FOUC (Flash Of Uninstantiated Components).
This commit fixes it by preparing the new document in an off-DOM node and
swapping the nodes when the embedded components are ready.

PR Close #18428

aio/src/app/layout/doc-viewer/doc-viewer.component.spec.ts

@@ -196,7 +196,7 @@ describe('DocViewerComponent', () => {
     const DOC_WITH_HIDDEN_H1_CONTENT = '<h1><i style="visibility: hidden">link</i>Features</h1>Some content';
 
     const tryDoc = (contents: string, docId = '') => {
-      docViewerEl.innerHTML = contents;
+      docViewer.currViewContainer.innerHTML = contents;
       docViewer.addTitleAndToc(docId);
     };
 
@@ -229,9 +229,10 @@ describe('DocViewerComponent', () => {
       });
 
       it('should fall back to `textContent` if `innerText` is not available', () => {
-        const querySelector_ = docViewerEl.querySelector;
+        const viewContainer = docViewer.currViewContainer;
-        spyOn(docViewerEl, 'querySelector').and.callFake((selector: string) => {
+        const querySelector_ = viewContainer.querySelector;
-          const elem = querySelector_.call(docViewerEl, selector);
+        spyOn(viewContainer, 'querySelector').and.callFake((selector: string) => {
+          const elem = querySelector_.call(viewContainer, selector);
           return Object.defineProperties(elem, {
             innerText: {value: undefined},
             textContent: {value: 'Text Content'},
@@ -244,9 +245,10 @@ describe('DocViewerComponent', () => {
       });
 
       it('should still use `innerText` if available but empty', () => {
-        const querySelector_ = docViewerEl.querySelector;
+        const viewContainer = docViewer.currViewContainer;
-        spyOn(docViewerEl, 'querySelector').and.callFake((selector: string) => {
+        const querySelector_ = viewContainer.querySelector;
-          const elem = querySelector_.call(docViewerEl, selector);
+        spyOn(viewContainer, 'querySelector').and.callFake((selector: string) => {
+          const elem = querySelector_.call(viewContainer, selector);
           return Object.defineProperties(elem, {
             innerText: { value: '' },
             textContent: { value: 'Text Content' }
@@ -273,7 +275,7 @@ describe('DocViewerComponent', () => {
         expect(tocEl).toBeTruthy();
         expect(tocEl.classList.contains('embedded')).toBe(true);
         expect(tocService.genToc).toHaveBeenCalledTimes(1);
-        expect(tocService.genToc).toHaveBeenCalledWith(docViewerEl, 'foo');
+        expect(tocService.genToc).toHaveBeenCalledWith(docViewer.currViewContainer, 'foo');
       });
 
       it('should have no ToC if there is a `.no-toc` `<h1>` heading', () => {
@@ -301,7 +303,7 @@ describe('DocViewerComponent', () => {
         tryDoc(DOC_WITH_H1, 'foo');
         expect(tocService.reset).toHaveBeenCalledTimes(1);
         expect(tocService.reset).toHaveBeenCalledBefore(tocService.genToc);
-        expect(tocService.genToc).toHaveBeenCalledWith(docViewerEl, 'foo');
+        expect(tocService.genToc).toHaveBeenCalledWith(docViewer.currViewContainer, 'foo');
 
         tocService.genToc.calls.reset();
         tryDoc(DOC_WITH_NO_TOC_H1, 'bar');
@@ -351,6 +353,7 @@ describe('DocViewerComponent', () => {
   describe('#render()', () => {
     let addTitleAndTocSpy: jasmine.Spy;
     let embedIntoSpy: jasmine.Spy;
+    let swapViewsSpy: jasmine.Spy;
 
     const doRender = (contents: string | null, id = 'foo') =>
       new Promise<void>((resolve, reject) =>
@@ -361,6 +364,7 @@ describe('DocViewerComponent', () => {
 
       addTitleAndTocSpy = spyOn(docViewer, 'addTitleAndToc');
       embedIntoSpy = embedComponentsService.embedInto.and.returnValue(of([]));
+      swapViewsSpy = spyOn(docViewer, 'swapViews').and.returnValue(of(undefined));
     });
 
     it('should return an `Observable`', () => {
@@ -368,40 +372,47 @@ describe('DocViewerComponent', () => {
     });
 
     describe('(contents, title, ToC)', () => {
+      beforeEach(() => swapViewsSpy.and.callThrough());
+
       it('should display the document contents', async () => {
         const contents = '<h1>Hello,</h1> <div>world!</div>';
         await doRender(contents);
 
-        expect(docViewerEl.innerHTML).toBe(contents);
+        expect(docViewerEl.innerHTML).toContain(contents);
+        expect(docViewerEl.textContent).toBe('Hello, world!');
       });
 
       it('should display nothing if the document has no contents', async () => {
-        docViewerEl.innerHTML = 'Test';
+        docViewer.currViewContainer.innerHTML = 'Test';
-        await doRender('');
+        expect(docViewerEl.textContent).toBe('Test');
-        expect(docViewerEl.innerHTML).toBe('');
+
+        await doRender('');
+        expect(docViewerEl.textContent).toBe('');
+
+        docViewer.currViewContainer.innerHTML = 'Test';
+        expect(docViewerEl.textContent).toBe('Test');
 
-        docViewerEl.innerHTML = 'Test';
         await doRender(null);
-        expect(docViewerEl.innerHTML).toBe('');
+        expect(docViewerEl.textContent).toBe('');
       });
 
       it('should set the title and ToC (after the content has been set)', async () => {
-        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.innerHTML).toBe('Foo content'));
+        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.textContent).toBe('Foo content'));
         await doRender('Foo content', 'foo');
         expect(addTitleAndTocSpy).toHaveBeenCalledTimes(1);
         expect(addTitleAndTocSpy).toHaveBeenCalledWith('foo');
 
-        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.innerHTML).toBe('Bar content'));
+        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.textContent).toBe('Bar content'));
         await doRender('Bar content', 'bar');
         expect(addTitleAndTocSpy).toHaveBeenCalledTimes(2);
         expect(addTitleAndTocSpy).toHaveBeenCalledWith('bar');
 
-        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.innerHTML).toBe(''));
+        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.textContent).toBe(''));
         await doRender('', 'baz');
         expect(addTitleAndTocSpy).toHaveBeenCalledTimes(3);
         expect(addTitleAndTocSpy).toHaveBeenCalledWith('baz');
 
-        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.innerHTML).toBe('Qux content'));
+        addTitleAndTocSpy.and.callFake(() => expect(docViewerEl.textContent).toBe('Qux content'));
         await doRender('Qux content', 'qux');
         expect(addTitleAndTocSpy).toHaveBeenCalledTimes(4);
         expect(addTitleAndTocSpy).toHaveBeenCalledWith('qux');
@@ -412,7 +423,7 @@ describe('DocViewerComponent', () => {
       it('should embed components', async () => {
         await doRender('Some content');
         expect(embedIntoSpy).toHaveBeenCalledTimes(1);
-        expect(embedIntoSpy).toHaveBeenCalledWith(docViewerEl);
+        expect(embedIntoSpy).toHaveBeenCalledWith(docViewer.nextViewContainer);
       });
 
       it('should attempt to embed components even if the document is empty', async () => {
@@ -420,8 +431,8 @@ describe('DocViewerComponent', () => {
         await doRender(null);
 
         expect(embedIntoSpy).toHaveBeenCalledTimes(2);
-        expect(embedIntoSpy.calls.argsFor(0)).toEqual([docViewerEl]);
+        expect(embedIntoSpy.calls.argsFor(0)).toEqual([docViewer.nextViewContainer]);
-        expect(embedIntoSpy.calls.argsFor(1)).toEqual([docViewerEl]);
+        expect(embedIntoSpy.calls.argsFor(1)).toEqual([docViewer.nextViewContainer]);
       });
 
       it('should store the embedded components', async () => {
@@ -450,36 +461,149 @@ describe('DocViewerComponent', () => {
       });
     });
 
-    describe('(on error) should log the error and recover', () => {
+    describe('(swapping views)', () => {
+      it('should swap the views after creating embedded components', async () => {
+        await doRender('<div></div>');
+
+        expect(swapViewsSpy).toHaveBeenCalledTimes(1);
+        expect(embedIntoSpy).toHaveBeenCalledBefore(swapViewsSpy);
+      });
+
+      it('should still swap the views if the document is empty', async () => {
+        await doRender('');
+        expect(swapViewsSpy).toHaveBeenCalledTimes(1);
+
+        await doRender(null);
+        expect(swapViewsSpy).toHaveBeenCalledTimes(2);
+      });
+
+      it('should unsubscribe from the previous "swap" observable when unsubscribed from', () => {
+        const obs = new ObservableWithSubscriptionSpies();
+        swapViewsSpy.and.returnValue(obs);
+
+        const renderObservable = docViewer.render({contents: 'Hello, world!', id: 'foo'});
+        const subscription = renderObservable.subscribe();
+
+        expect(obs.subscribeSpy).toHaveBeenCalledTimes(1);
+        expect(obs.unsubscribeSpies[0]).not.toHaveBeenCalled();
+
+        subscription.unsubscribe();
+
+        expect(obs.subscribeSpy).toHaveBeenCalledTimes(1);
+        expect(obs.unsubscribeSpies[0]).toHaveBeenCalledTimes(1);
+      });
+    });
+
+    describe('(on error) should clean up, log the error and recover', () => {
       let logger: MockLogger;
 
       beforeEach(() => logger = TestBed.get(Logger));
 
-      it('when `addTitleAndToc()` fails', async () => {
+      it('when `EmbedComponentsService.embedInto()` fails', async () => {
-        const error = Error('Typical `addTitleAndToc()` error');
+        const error = Error('Typical `embedInto()` error');
-        addTitleAndTocSpy.and.callFake(() => { throw error; });
+        embedIntoSpy.and.callFake(() => {
+          expect(docViewer.nextViewContainer.innerHTML).not.toBe('');
+          throw error;
+        });
 
         await doRender('Some content', 'foo');
 
-        expect(addTitleAndTocSpy).toHaveBeenCalledTimes(1);
+        expect(embedIntoSpy).toHaveBeenCalledTimes(1);
-        expect(embedIntoSpy).not.toHaveBeenCalled();
+        expect(swapViewsSpy).not.toHaveBeenCalled();
+        expect(addTitleAndTocSpy).not.toHaveBeenCalled();
+        expect(docViewer.nextViewContainer.innerHTML).toBe('');
         expect(logger.output.error).toEqual([
           ['[DocViewer]: Error preparing document \'foo\'.', error],
         ]);
       });
 
-      it('when `EmbedComponentsService#embedInto()` fails', async () => {
+      it('when `swapViews()` fails', async () => {
-        const error = Error('Typical `embedInto()` error');
+        const error = Error('Typical `swapViews()` error');
-        embedIntoSpy.and.callFake(() => { throw error; });
+        swapViewsSpy.and.callFake(() => {
+          expect(docViewer.nextViewContainer.innerHTML).not.toBe('');
+          throw error;
+        });
 
         await doRender('Some content', 'bar');
 
-        expect(addTitleAndTocSpy).toHaveBeenCalledTimes(1);
         expect(embedIntoSpy).toHaveBeenCalledTimes(1);
+        expect(swapViewsSpy).toHaveBeenCalledTimes(1);
+        expect(addTitleAndTocSpy).not.toHaveBeenCalled();
+        expect(docViewer.nextViewContainer.innerHTML).toBe('');
         expect(logger.output.error).toEqual([
           ['[DocViewer]: Error preparing document \'bar\'.', error],
         ]);
       });
+
+      it('when `addTitleAndTocSpy()` fails', async () => {
+        const error = Error('Typical `addTitleAndToc()` error');
+        addTitleAndTocSpy.and.callFake(() => {
+          expect(docViewer.nextViewContainer.innerHTML).not.toBe('');
+          throw error;
+        });
+
+        await doRender('Some content', 'baz');
+
+        expect(embedIntoSpy).toHaveBeenCalledTimes(1);
+        expect(swapViewsSpy).toHaveBeenCalledTimes(1);
+        expect(addTitleAndTocSpy).toHaveBeenCalledTimes(1);
+        expect(docViewer.nextViewContainer.innerHTML).toBe('');
+        expect(logger.output.error).toEqual([
+          ['[DocViewer]: Error preparing document \'baz\'.', error],
+        ]);
+      });
+    });
+  });
+
+  describe('#swapViews()', () => {
+    let oldCurrViewContainer: HTMLElement;
+    let oldNextViewContainer: HTMLElement;
+
+    const doSwapViews = () => new Promise<void>((resolve, reject) =>
+      docViewer.swapViews().subscribe(resolve, reject));
+
+    beforeEach(() => {
+      oldCurrViewContainer = docViewer.currViewContainer;
+      oldNextViewContainer = docViewer.nextViewContainer;
+
+      oldCurrViewContainer.innerHTML = 'Current view';
+      oldNextViewContainer.innerHTML = 'Next view';
+
+      expect(docViewerEl.contains(oldCurrViewContainer)).toBe(true);
+      expect(docViewerEl.contains(oldNextViewContainer)).toBe(false);
+    });
+
+    it('should return an observable', done => {
+      docViewer.swapViews().subscribe(done, done.fail);
+    });
+
+    it('should swap the views', async () => {
+      await doSwapViews();
+
+      expect(docViewerEl.contains(oldCurrViewContainer)).toBe(false);
+      expect(docViewerEl.contains(oldNextViewContainer)).toBe(true);
+      expect(docViewer.currViewContainer).toBe(oldNextViewContainer);
+      expect(docViewer.nextViewContainer).toBe(oldCurrViewContainer);
+
+      await doSwapViews();
+
+      expect(docViewerEl.contains(oldCurrViewContainer)).toBe(true);
+      expect(docViewerEl.contains(oldNextViewContainer)).toBe(false);
+      expect(docViewer.currViewContainer).toBe(oldCurrViewContainer);
+      expect(docViewer.nextViewContainer).toBe(oldNextViewContainer);
+    });
+
+    it('should empty the previous view', async () => {
+      await doSwapViews();
+
+      expect(docViewer.currViewContainer.innerHTML).toBe('Next view');
+      expect(docViewer.nextViewContainer.innerHTML).toBe('');
+
+      docViewer.nextViewContainer.innerHTML = 'Next view 2';
+      await doSwapViews();
+
+      expect(docViewer.currViewContainer.innerHTML).toBe('Next view 2');
+      expect(docViewer.nextViewContainer.innerHTML).toBe('');
     });
   });
 });

aio/src/app/layout/doc-viewer/doc-viewer.component.ts

@@ -33,6 +33,8 @@ export class DocViewerComponent implements DoCheck, OnDestroy {
   private docContents$ = new EventEmitter<DocumentContents>();
 
   protected embeddedComponentRefs: ComponentRef<any>[] = [];
+  protected currViewContainer: HTMLElement = document.createElement('div');
+  protected nextViewContainer: HTMLElement = document.createElement('div');
 
   @Input()
   set doc(newDoc: DocumentContents) {
@@ -57,6 +59,12 @@ export class DocViewerComponent implements DoCheck, OnDestroy {
     // Security: the initialDocViewerContent comes from the prerendered DOM and is considered to be secure
     this.hostElement.innerHTML = initialDocViewerContent;
 
+    if (this.hostElement.firstElementChild) {
+      this.currViewContainer = this.hostElement.firstElementChild as HTMLElement;
+    } else {
+      this.hostElement.appendChild(this.currViewContainer);
+    }
+
     this.onDestroy$.subscribe(() => this.destroyEmbeddedComponents());
     this.docContents$
         .do(() => this.destroyEmbeddedComponents())
@@ -79,7 +87,7 @@ export class DocViewerComponent implements DoCheck, OnDestroy {
    */
   protected addTitleAndToc(docId: string): void {
     this.tocService.reset();
-    const titleEl = this.hostElement.querySelector('h1');
+    const titleEl = this.currViewContainer.querySelector('h1');
     let title = '';
 
     // Only create TOC for docs with an <h1> title
@@ -87,7 +95,7 @@ export class DocViewerComponent implements DoCheck, OnDestroy {
     if (titleEl) {
       title = (typeof titleEl.innerText === 'string') ? titleEl.innerText : titleEl.textContent;
       if (!/(no-toc|notoc)/i.test(titleEl.className)) {
-        this.tocService.genToc(this.hostElement, docId);
+        this.tocService.genToc(this.currViewContainer, docId);
         titleEl.insertAdjacentHTML('afterend', '<aio-toc class="embedded"></aio-toc>');
       }
     }
@@ -111,15 +119,48 @@ export class DocViewerComponent implements DoCheck, OnDestroy {
         .do(() => {
           // Security: `doc.contents` is always authored by the documentation team
           //           and is considered to be safe.
-          this.hostElement.innerHTML = doc.contents || '';
+          this.nextViewContainer.innerHTML = doc.contents || '';
-          this.addTitleAndToc(doc.id);
         })
-        .switchMap(() => this.embedComponentsService.embedInto(this.hostElement))
+        .switchMap(() => this.embedComponentsService.embedInto(this.nextViewContainer))
         .do(componentRefs => this.embeddedComponentRefs = componentRefs)
-        .switchMap(() => this.void$)
+        .switchMap(() => this.swapViews())
+        .do(() => this.addTitleAndToc(doc.id))
         .catch(err => {
+          this.nextViewContainer.innerHTML = '';
           this.logger.error(`[DocViewer]: Error preparing document '${doc.id}'.`, err);
           return this.void$;
         });
   }
+
+  /**
+   * Swap the views, removing `currViewContainer` and inserting `nextViewContainer`.
+   * (At this point all content should be ready, including having loaded and instantiated embedded
+   *  components.)
+   */
+  protected swapViews(): Observable<void> {
+    // Placeholders for actual animations.
+    const animateLeave = (elem: HTMLElement) => this.void$;
+    const animateEnter = (elem: HTMLElement) => this.void$;
+
+    let done$ = this.void$;
+
+    if (this.currViewContainer.parentElement) {
+      done$ = done$
+          // Remove the current view from the viewer.
+          .switchMap(() => animateLeave(this.currViewContainer))
+          .do(() => this.currViewContainer.parentElement.removeChild(this.currViewContainer));
+    }
+
+    return done$
+        // Insert the next view into the viewer.
+        .do(() => this.hostElement.appendChild(this.nextViewContainer))
+        .switchMap(() => animateEnter(this.nextViewContainer))
+        // Update the view references and clean up unused nodes.
+        .do(() => {
+          const prevViewContainer = this.currViewContainer;
+          this.currViewContainer = this.nextViewContainer;
+          this.nextViewContainer = prevViewContainer;
+          this.nextViewContainer.innerHTML = '';  // Empty to release memory.
+        });
+  }
 }

aio/src/testing/doc-viewer-utils.ts

@@ -18,10 +18,13 @@ import { MockLogger } from 'testing/logger.service';
 
 export class TestDocViewerComponent extends DocViewerComponent {
   embeddedComponentRefs: ComponentRef<any>[];
+  currViewContainer: HTMLElement;
+  nextViewContainer: HTMLElement;
 
   addTitleAndToc(docId: string): void { return null as any; }
   destroyEmbeddedComponents(): void { return null as any; }
   render(doc: DocumentContents): Observable<void> { return null as any; }
+  swapViews(): Observable<void> { return null as any; }
 }