From ae86cb3be0c413544f8b17cfcd0e29ae21655b4c Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 15 May 2018 19:41:13 +0300 Subject: [PATCH] fix(aio): do not load custom elements again while already loading (#23944) PR Close #23944 --- aio/scripts/_payload-limits.json | 2 +- .../custom-elements/elements-loader.spec.ts | 282 +++++++++++++----- .../app/custom-elements/elements-loader.ts | 64 ++-- 3 files changed, 251 insertions(+), 97 deletions(-) diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json index ac9b44a215..a3fdfa790d 100755 --- a/aio/scripts/_payload-limits.json +++ b/aio/scripts/_payload-limits.json @@ -3,7 +3,7 @@ "master": { "uncompressed": { "runtime": 2712, - "main": 479417, + "main": 479729, "polyfills": 38453, "prettify": 14913 } diff --git a/aio/src/app/custom-elements/elements-loader.spec.ts b/aio/src/app/custom-elements/elements-loader.spec.ts index 1fc4eed22c..906e74283e 100644 --- a/aio/src/app/custom-elements/elements-loader.spec.ts +++ b/aio/src/app/custom-elements/elements-loader.spec.ts @@ -4,49 +4,19 @@ import { NgModuleRef, Type } from '@angular/core'; -import {TestBed, fakeAsync, tick} from '@angular/core/testing'; +import { TestBed, fakeAsync, flushMicrotasks } from '@angular/core/testing'; import { ElementsLoader } from './elements-loader'; import { ELEMENT_MODULE_PATHS_TOKEN, WithCustomElementComponent } from './element-registry'; -class FakeComponentFactory extends ComponentFactory { - selector: string; - componentType: Type; - ngContentSelectors: string[]; - inputs = [{propName: this.identifyingInput, templateName: this.identifyingInput}]; - outputs = []; - constructor(private identifyingInput: string) { super(); } - - create(injector: Injector, - projectableNodes?: any[][], - rootSelectorOrNode?: string | any, - ngModule?: NgModuleRef): ComponentRef { - return (jasmine.createSpy('ComponentRef') as any) as ComponentRef; - }; +interface Deferred { + resolve(): void; + reject(err: any): void; } -const FAKE_COMPONENT_FACTORIES = new Map([ - ['element-a-module-path', new FakeComponentFactory('element-a-input')], - ['element-b-module-path', new FakeComponentFactory('element-b-input')], -]); - describe('ElementsLoader', () => { let elementsLoader: ElementsLoader; - let actualCustomElementsDefine; - let fakeCustomElementsDefine; - - // ElementsLoader uses the window's customElements API. Provide a fake for this test. - beforeEach(() => { - actualCustomElementsDefine = window.customElements.define; - - fakeCustomElementsDefine = jasmine.createSpy('define'); - - window.customElements.define = fakeCustomElementsDefine; - }); - afterEach(() => { - window.customElements.define = actualCustomElementsDefine; - }); beforeEach(() => { const injector = TestBed.configureTestingModule({ @@ -63,63 +33,196 @@ describe('ElementsLoader', () => { elementsLoader = injector.get(ElementsLoader); }); - it('should be able to register an element', fakeAsync(() => { - // Verify that the elements loader considered `element-a-selector` to be unregistered. - expect(elementsLoader.elementsToLoad.has('element-a-selector')).toBeTruthy(); + describe('loadContainingCustomElements()', () => { + let loadCustomElementSpy: jasmine.Spy; - const hostEl = document.createElement('div'); - hostEl.innerHTML = ``; + beforeEach(() => loadCustomElementSpy = spyOn(elementsLoader, 'loadCustomElement')); - elementsLoader.loadContainingCustomElements(hostEl); - tick(); + it('should attempt to load and register all contained elements', fakeAsync(() => { + expect(loadCustomElementSpy).not.toHaveBeenCalled(); - const defineArgs = fakeCustomElementsDefine.calls.argsFor(0); - expect(defineArgs[0]).toBe('element-a-selector'); + const hostEl = document.createElement('div'); + hostEl.innerHTML = ` + + + `; - // Verify the right component was loaded/created - expect(defineArgs[1].observedAttributes[0]).toBe('element-a-input'); + elementsLoader.loadContainingCustomElements(hostEl); + flushMicrotasks(); - expect(elementsLoader.elementsToLoad.has('element-a-selector')).toBeFalsy(); - })); + expect(loadCustomElementSpy).toHaveBeenCalledTimes(2); + expect(loadCustomElementSpy).toHaveBeenCalledWith('element-a-selector'); + expect(loadCustomElementSpy).toHaveBeenCalledWith('element-b-selector'); + })); - it('should be able to register multiple elements', fakeAsync(() => { - // Verify that the elements loader considered `element-a-selector` to be unregistered. - expect(elementsLoader.elementsToLoad.has('element-a-selector')).toBeTruthy(); + it('should attempt to load and register only contained elements', fakeAsync(() => { + expect(loadCustomElementSpy).not.toHaveBeenCalled(); - const hostEl = document.createElement('div'); - hostEl.innerHTML = ` - - - `; + const hostEl = document.createElement('div'); + hostEl.innerHTML = ` + + `; - elementsLoader.loadContainingCustomElements(hostEl); - tick(); + elementsLoader.loadContainingCustomElements(hostEl); + flushMicrotasks(); - const defineElementA = fakeCustomElementsDefine.calls.argsFor(0); - expect(defineElementA[0]).toBe('element-a-selector'); - expect(defineElementA[1].observedAttributes[0]).toBe('element-a-input'); - expect(elementsLoader.elementsToLoad.has('element-a-selector')).toBeFalsy(); + expect(loadCustomElementSpy).toHaveBeenCalledTimes(1); + expect(loadCustomElementSpy).toHaveBeenCalledWith('element-b-selector'); + })); - const defineElementB = fakeCustomElementsDefine.calls.argsFor(1); - expect(defineElementB[0]).toBe('element-b-selector'); - expect(defineElementB[1].observedAttributes[0]).toBe('element-b-input'); - expect(elementsLoader.elementsToLoad.has('element-b-selector')).toBeFalsy(); - })); + it('should wait for all contained elements to load and register', fakeAsync(() => { + const deferreds = returnPromisesFromSpy(loadCustomElementSpy); - it('should only register an element one time', fakeAsync(() => { - const hostEl = document.createElement('div'); - hostEl.innerHTML = ``; + const hostEl = document.createElement('div'); + hostEl.innerHTML = ` + + + `; - elementsLoader.loadContainingCustomElements(hostEl); - tick(); // Tick for the module factory loader's async `load` function + const log: any[] = []; + elementsLoader.loadContainingCustomElements(hostEl).subscribe( + v => log.push(`emitted: ${v}`), + e => log.push(`errored: ${e}`), + () => log.push('completed'), + ); - // Call again to to check how many times customElements.define was called. - elementsLoader.loadContainingCustomElements(hostEl); - tick(); // Tick for the module factory loader's async `load` function + flushMicrotasks(); + expect(log).toEqual([]); - // Should have only been called once, since the second load would not query for element-a - expect(window.customElements.define).toHaveBeenCalledTimes(1); - })); + deferreds[0].resolve(); + flushMicrotasks(); + expect(log).toEqual([]); + + deferreds[1].resolve(); + flushMicrotasks(); + expect(log).toEqual(['emitted: undefined', 'completed']); + })); + + it('should fail if any of the contained elements fails to load and register', fakeAsync(() => { + const deferreds = returnPromisesFromSpy(loadCustomElementSpy); + + const hostEl = document.createElement('div'); + hostEl.innerHTML = ` + + + `; + + const log: any[] = []; + elementsLoader.loadContainingCustomElements(hostEl).subscribe( + v => log.push(`emitted: ${v}`), + e => log.push(`errored: ${e}`), + () => log.push('completed'), + ); + + flushMicrotasks(); + expect(log).toEqual([]); + + deferreds[0].resolve(); + flushMicrotasks(); + expect(log).toEqual([]); + + deferreds[1].reject('foo'); + flushMicrotasks(); + expect(log).toEqual(['errored: foo']); + })); + }); + + describe('loadCustomElement()', () => { + let definedSpy: jasmine.Spy; + let whenDefinedSpy: jasmine.Spy; + let whenDefinedDeferreds: Deferred[]; + + beforeEach(() => { + // `loadCustomElement()` uses the `window.customElements` API. Provide mocks for these tests. + definedSpy = spyOn(window.customElements, 'define'); + whenDefinedSpy = spyOn(window.customElements, 'whenDefined'); + whenDefinedDeferreds = returnPromisesFromSpy(whenDefinedSpy); + }); + + it('should be able to load and register an element', fakeAsync(() => { + elementsLoader.loadCustomElement('element-a-selector'); + flushMicrotasks(); + + expect(definedSpy).toHaveBeenCalledTimes(1); + expect(definedSpy).toHaveBeenCalledWith('element-a-selector', jasmine.any(Function)); + + // Verify the right component was loaded/registered. + const Ctor = definedSpy.calls.argsFor(0)[1]; + expect(Ctor.observedAttributes).toEqual(['element-a-module-path']); + })); + + it('should wait until the element is defined', fakeAsync(() => { + let state = 'pending'; + elementsLoader.loadCustomElement('element-b-selector').then(() => state = 'resolved'); + flushMicrotasks(); + + expect(state).toBe('pending'); + expect(whenDefinedSpy).toHaveBeenCalledTimes(1); + expect(whenDefinedSpy).toHaveBeenCalledWith('element-b-selector'); + + whenDefinedDeferreds[0].resolve(); + flushMicrotasks(); + expect(state).toBe('resolved'); + })); + + it('should not load and register the same element more than once', fakeAsync(() => { + elementsLoader.loadCustomElement('element-a-selector'); + flushMicrotasks(); + expect(definedSpy).toHaveBeenCalledTimes(1); + + definedSpy.calls.reset(); + + // While loading/registering is still in progress: + elementsLoader.loadCustomElement('element-a-selector'); + flushMicrotasks(); + expect(definedSpy).not.toHaveBeenCalled(); + + definedSpy.calls.reset(); + whenDefinedDeferreds[0].resolve(); + + // Once loading/registering is already completed: + let state = 'pending'; + elementsLoader.loadCustomElement('element-a-selector').then(() => state = 'resolved'); + flushMicrotasks(); + expect(state).toBe('resolved'); + expect(definedSpy).not.toHaveBeenCalled(); + })); + + it('should fail if defining the the custom element fails', fakeAsync(() => { + let state = 'pending'; + elementsLoader.loadCustomElement('element-b-selector').catch(e => state = `rejected: ${e}`); + flushMicrotasks(); + expect(state).toBe('pending'); + + whenDefinedDeferreds[0].reject('foo'); + flushMicrotasks(); + expect(state).toBe('rejected: foo'); + })); + + it('should be able to load and register an element again if previous attempt failed', + fakeAsync(() => { + elementsLoader.loadCustomElement('element-a-selector'); + flushMicrotasks(); + expect(definedSpy).toHaveBeenCalledTimes(1); + + definedSpy.calls.reset(); + + // While loading/registering is still in progress: + elementsLoader.loadCustomElement('element-a-selector').catch(() => undefined); + flushMicrotasks(); + expect(definedSpy).not.toHaveBeenCalled(); + + whenDefinedDeferreds[0].reject('foo'); + flushMicrotasks(); + expect(definedSpy).not.toHaveBeenCalled(); + + // Once loading/registering has already failed: + elementsLoader.loadCustomElement('element-a-selector'); + flushMicrotasks(); + expect(definedSpy).toHaveBeenCalledTimes(1); + }) + ); + }); }); // TEST CLASSES/HELPERS @@ -128,11 +231,28 @@ class FakeCustomElementModule implements WithCustomElementComponent { customElementComponent: Type; } +class FakeComponentFactory extends ComponentFactory { + selector: string; + componentType: Type; + ngContentSelectors: string[]; + inputs = [{propName: this.identifyingInput, templateName: this.identifyingInput}]; + outputs = []; + + constructor(private identifyingInput: string) { super(); } + + create(injector: Injector, + projectableNodes?: any[][], + rootSelectorOrNode?: string | any, + ngModule?: NgModuleRef): ComponentRef { + return jasmine.createSpy('ComponentRef') as any; + }; +} + class FakeComponentFactoryResolver extends ComponentFactoryResolver { constructor(private modulePath) { super(); } resolveComponentFactory(component: Type): ComponentFactory { - return FAKE_COMPONENT_FACTORIES.get(this.modulePath)!; + return new FakeComponentFactory(this.modulePath); } } @@ -168,3 +288,9 @@ class FakeModuleFactoryLoader extends NgModuleFactoryLoader { return Promise.resolve(fakeModuleFactory); } } + +function returnPromisesFromSpy(spy: jasmine.Spy): Deferred[] { + const deferreds: Deferred[] = []; + spy.and.callFake(() => new Promise((resolve, reject) => deferreds.push({resolve, reject}))); + return deferreds; +} diff --git a/aio/src/app/custom-elements/elements-loader.ts b/aio/src/app/custom-elements/elements-loader.ts index 1316608e50..0dd45a008d 100644 --- a/aio/src/app/custom-elements/elements-loader.ts +++ b/aio/src/app/custom-elements/elements-loader.ts @@ -11,11 +11,13 @@ import { createCustomElement } from '@angular/elements'; @Injectable() export class ElementsLoader { /** Map of unregistered custom elements and their respective module paths to load. */ - elementsToLoad: Map; + private elementsToLoad: Map; + /** Map of custom elements that are in the process of being loaded and registered. */ + private elementsLoading = new Map>(); constructor(private moduleFactoryLoader: NgModuleFactoryLoader, private moduleRef: NgModuleRef, - @Inject(ELEMENT_MODULE_PATHS_TOKEN) elementModulePaths) { + @Inject(ELEMENT_MODULE_PATHS_TOKEN) elementModulePaths: Map) { this.elementsToLoad = new Map(elementModulePaths); } @@ -25,30 +27,56 @@ export class ElementsLoader { * elements so that they will not be queried in subsequent calls. */ loadContainingCustomElements(element: HTMLElement): Observable { - const selectors: any[] = Array.from(this.elementsToLoad.keys()) + const unregisteredSelectors = Array.from(this.elementsToLoad.keys()) .filter(s => element.querySelector(s)); - if (!selectors.length) { return of(undefined); } + if (!unregisteredSelectors.length) { return of(undefined); } // Returns observable that completes when all discovered elements have been registered. - return fromPromise(Promise.all(selectors.map(s => this.register(s))).then(result => undefined)); + const allRegistered = Promise.all(unregisteredSelectors.map(s => this.loadCustomElement(s))); + return fromPromise(allRegistered.then(() => undefined)); } - /** Registers the custom element defined on the WithCustomElement module factory. */ - private register(selector: string) { - const modulePath = this.elementsToLoad.get(selector)!; - return this.moduleFactoryLoader.load(modulePath).then(elementModuleFactory => { - if (!this.elementsToLoad.has(selector)) { return; } + /** Loads and registers the custom element defined on the `WithCustomElement` module factory. */ + loadCustomElement(selector: string): Promise { + if (this.elementsLoading.has(selector)) { + // The custom element is in the process of being loaded and registered. + return this.elementsLoading.get(selector)!; + } - const elementModuleRef = elementModuleFactory.create(this.moduleRef.injector); - const CustomElementComponent = elementModuleRef.instance.customElementComponent; - const CustomElement = - createCustomElement(CustomElementComponent, {injector: elementModuleRef.injector}); + if (this.elementsToLoad.has(selector)) { + // Load and register the custom element (for the first time). + const modulePath = this.elementsToLoad.get(selector)!; + const loadedAndRegistered = this.moduleFactoryLoader + .load(modulePath) + .then(elementModuleFactory => { + const elementModuleRef = elementModuleFactory.create(this.moduleRef.injector); + const injector = elementModuleRef.injector; + const CustomElementComponent = elementModuleRef.instance.customElementComponent; + const CustomElement = createCustomElement(CustomElementComponent, {injector}); - customElements!.define(selector, CustomElement); - this.elementsToLoad.delete(selector); + customElements!.define(selector, CustomElement); + return customElements.whenDefined(selector); + }) + .then(() => { + // The custom element has been successfully loaded and registered. + // Remove from `elementsLoading` and `elementsToLoad`. + this.elementsLoading.delete(selector); + this.elementsToLoad.delete(selector); + }) + .catch(err => { + // The custom element has failed to load and register. + // Remove from `elementsLoading`. + // (Do not remove from `elementsToLoad` in case it was a temporary error.) + this.elementsLoading.delete(selector); + return Promise.reject(err); + }); - return customElements.whenDefined(selector); - }); + this.elementsLoading.set(selector, loadedAndRegistered); + return loadedAndRegistered; + } + + // The custom element has already been loaded and registered. + return Promise.resolve(); } }