diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index 3cefa0c90f..f92c20b32d 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 3037, - "main-es2015": 448615, + "main-es2015": 448922, "polyfills-es2015": 52415 } } @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime-es2015": 3157, - "main-es2015": 431750, + "main-es2015": 432199, "polyfills-es2015": 52415 } } diff --git a/integration/ng_elements/e2e/app.e2e-spec.ts b/integration/ng_elements/e2e/app.e2e-spec.ts index 8da6d63c43..0d7e65ffbc 100644 --- a/integration/ng_elements/e2e/app.e2e-spec.ts +++ b/integration/ng_elements/e2e/app.e2e-spec.ts @@ -5,7 +5,7 @@ describe('Element E2E Tests', function () { describe('Hello World Elements', () => { beforeEach(() => browser.get('hello-world.html')); - describe('(with default view encapsulation)', () => { + describe('(with default CD strategy and view encapsulation)', () => { const helloWorldEl = element(by.css('hello-world-el')); it('should display "Hello World!"', function () { @@ -21,6 +21,22 @@ describe('Element E2E Tests', function () { }); }); + describe('(with `OnPush` CD strategy)', () => { + const helloWorldOnpushEl = element(by.css('hello-world-onpush-el')); + + it('should display "Hello World!"', function () { + expect(helloWorldOnpushEl.getText()).toBe('Hello World!'); + }); + + it('should display "Hello Foo!" via name attribute', function () { + const input = element(by.css('input[type=text]')); + input.sendKeys('Foo'); + + // Make tests less flaky on CI by waiting up to 5s for the element text to be updated. + browser.wait(EC.textToBePresentInElement(helloWorldOnpushEl, 'Hello Foo!'), 5000); + }); + }); + describe('(with `ShadowDom` view encapsulation)', () => { const helloWorldShadowEl = element(by.css('hello-world-shadow-el')); const getShadowDomText = (el: ElementFinder) => diff --git a/integration/ng_elements/src/app.ts b/integration/ng_elements/src/app.ts index 23dbddb997..8064cce22f 100644 --- a/integration/ng_elements/src/app.ts +++ b/integration/ng_elements/src/app.ts @@ -2,17 +2,29 @@ import {Injector, NgModule} from '@angular/core'; import {createCustomElement} from '@angular/elements'; import {BrowserModule} from '@angular/platform-browser'; -import {HelloWorldComponent, HelloWorldShadowComponent, TestCardComponent} from './elements'; +import {HelloWorldComponent, HelloWorldOnpushComponent, HelloWorldShadowComponent, TestCardComponent} from './elements'; @NgModule({ - declarations: [HelloWorldComponent, HelloWorldShadowComponent, TestCardComponent], - entryComponents: [HelloWorldComponent, HelloWorldShadowComponent, TestCardComponent], + declarations: [ + HelloWorldComponent, + HelloWorldOnpushComponent, + HelloWorldShadowComponent, + TestCardComponent, + ], + entryComponents: [ + HelloWorldComponent, + HelloWorldOnpushComponent, + HelloWorldShadowComponent, + TestCardComponent, + ], imports: [BrowserModule], }) export class AppModule { constructor(injector: Injector) { customElements.define('hello-world-el', createCustomElement(HelloWorldComponent, {injector})); + customElements.define( + 'hello-world-onpush-el', createCustomElement(HelloWorldOnpushComponent, {injector})); customElements.define( 'hello-world-shadow-el', createCustomElement(HelloWorldShadowComponent, {injector})); customElements.define('test-card', createCustomElement(TestCardComponent, {injector})); diff --git a/integration/ng_elements/src/elements.ts b/integration/ng_elements/src/elements.ts index 33276decda..e3252e7761 100644 --- a/integration/ng_elements/src/elements.ts +++ b/integration/ng_elements/src/elements.ts @@ -1,4 +1,4 @@ -import {Component, Input, ViewEncapsulation} from '@angular/core'; +import {ChangeDetectionStrategy, Component, Input, ViewEncapsulation} from '@angular/core'; @Component({ selector: 'hello-world-el', @@ -8,6 +8,15 @@ export class HelloWorldComponent { @Input() name: string = 'World'; } +@Component({ + selector: 'hello-world-onpush-el', + template: 'Hello {{name}}!', + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class HelloWorldOnpushComponent { + @Input() name: string = 'World'; +} + @Component({ selector: 'hello-world-shadow-el', template: 'Hello {{name}}!', diff --git a/integration/ng_elements/src/hello-world.html b/integration/ng_elements/src/hello-world.html index 022a84790a..d9b2136e25 100644 --- a/integration/ng_elements/src/hello-world.html +++ b/integration/ng_elements/src/hello-world.html @@ -10,6 +10,7 @@ + diff --git a/integration/ng_elements/src/main.ts b/integration/ng_elements/src/main.ts index f700992f2f..608f5bd5aa 100644 --- a/integration/ng_elements/src/main.ts +++ b/integration/ng_elements/src/main.ts @@ -5,9 +5,11 @@ platformBrowser().bootstrapModuleFactory(AppModuleNgFactory, {ngZone: 'noop'}); const input = document.querySelector('input')!; const helloWorld = document.querySelector('hello-world-el')!; +const helloWorldOnpush = document.querySelector('hello-world-onpush-el')!; const helloWorldShadow = document.querySelector('hello-world-shadow-el')!; input.addEventListener('input', () => { helloWorld.setAttribute('name', input.value); + helloWorldOnpush.setAttribute('name', input.value); helloWorldShadow.setAttribute('name', input.value); }); diff --git a/packages/elements/src/component-factory-strategy.ts b/packages/elements/src/component-factory-strategy.ts index 35db08958a..0d013c26b5 100644 --- a/packages/elements/src/component-factory-strategy.ts +++ b/packages/elements/src/component-factory-strategy.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ApplicationRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, EventEmitter, Injector, NgZone, OnChanges, SimpleChange, SimpleChanges, Type} from '@angular/core'; +import {ApplicationRef, ChangeDetectorRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, EventEmitter, Injector, NgZone, OnChanges, SimpleChange, SimpleChanges, Type} from '@angular/core'; import {merge, Observable, ReplaySubject} from 'rxjs'; import {map, switchMap} from 'rxjs/operators'; @@ -52,10 +52,19 @@ export class ComponentNgElementStrategy implements NgElementStrategy { /** Reference to the component that was created on connect. */ private componentRef: ComponentRef|null = null; - /** Changes that have been made to the component ref since the last time onChanges was called. */ + /** Reference to the component view's `ChangeDetectorRef`. */ + private viewChangeDetectorRef: ChangeDetectorRef|null = null; + + /** + * Changes that have been made to component inputs since the last change detection run. + * (NOTE: These are only recorded if the component implements the `OnChanges` interface.) + */ private inputChanges: SimpleChanges|null = null; - /** Whether the created component implements the onChanges function. */ + /** Whether changes have been made to component inputs since the last change detection run. */ + private hasInputChanges = false; + + /** Whether the created component implements the `OnChanges` interface. */ private implementsOnChanges = false; /** Whether a change detection has been scheduled to run on the component. */ @@ -68,10 +77,12 @@ export class ComponentNgElementStrategy implements NgElementStrategy { private readonly initialInputValues = new Map(); /** - * Set of component inputs that have not yet changed, i.e. for which `ngOnChanges()` has not - * fired. (This is used to determine the value of `fistChange` in `SimpleChange` instances.) + * Set of component inputs that have not yet changed, i.e. for which `recordInputChange()` has not + * fired. + * (This helps detect the first change of an input, even if it is explicitly set to `undefined`.) */ - private readonly unchangedInputs = new Set(); + private readonly unchangedInputs = + new Set(this.componentFactory.inputs.map(({propName}) => propName)); /** Service for setting zone context. */ private readonly ngZone = this.injector.get(NgZone); @@ -119,6 +130,7 @@ export class ComponentNgElementStrategy implements NgElementStrategy { if (this.componentRef !== null) { this.componentRef.destroy(); this.componentRef = null; + this.viewChangeDetectorRef = null; } }, DESTROY_DELAY); }); @@ -157,7 +169,13 @@ export class ComponentNgElementStrategy implements NgElementStrategy { return; } + // Record the changed value and update internal state to reflect the fact that this input has + // changed. this.recordInputChange(property, value); + this.unchangedInputs.delete(property); + this.hasInputChanges = true; + + // Update the component instance and schedule change detection. this.componentRef.instance[property] = value; this.scheduleDetectChanges(); }); @@ -172,6 +190,7 @@ export class ComponentNgElementStrategy implements NgElementStrategy { const projectableNodes = extractProjectableNodes(element, this.componentFactory.ngContentSelectors); this.componentRef = this.componentFactory.create(childInjector, projectableNodes, element); + this.viewChangeDetectorRef = this.componentRef.injector.get(ChangeDetectorRef); this.implementsOnChanges = isFunction((this.componentRef.instance as OnChanges).ngOnChanges); @@ -187,12 +206,6 @@ export class ComponentNgElementStrategy implements NgElementStrategy { /** Set any stored initial inputs on the component's properties. */ protected initializeInputs(): void { this.componentFactory.inputs.forEach(({propName}) => { - if (this.implementsOnChanges) { - // If the component implements `ngOnChanges()`, keep track of which inputs have never - // changed so far. - this.unchangedInputs.add(propName); - } - if (this.initialInputValues.has(propName)) { // Call `setInputValue()` now that the component has been instantiated to update its // properties and fire `ngOnChanges()`. @@ -227,6 +240,17 @@ export class ComponentNgElementStrategy implements NgElementStrategy { (componentRef.instance as OnChanges).ngOnChanges(inputChanges); } + /** + * Marks the component view for check, if necessary. + * (NOTE: This is required when the `ChangeDetectionStrategy` is set to `OnPush`.) + */ + protected markViewForCheck(viewChangeDetectorRef: ChangeDetectorRef): void { + if (this.hasInputChanges) { + this.hasInputChanges = false; + viewChangeDetectorRef.markForCheck(); + } + } + /** * Schedules change detection to run on the component. * Ignores subsequent calls if already scheduled. @@ -247,8 +271,7 @@ export class ComponentNgElementStrategy implements NgElementStrategy { */ protected recordInputChange(property: string, currentValue: any): void { // Do not record the change if the component does not implement `OnChanges`. - // (We can only determine that after the component has been instantiated.) - if (this.componentRef !== null && !this.implementsOnChanges) { + if (!this.implementsOnChanges) { return; } @@ -257,7 +280,7 @@ export class ComponentNgElementStrategy implements NgElementStrategy { } // If there already is a change, modify the current value to match but leave the values for - // previousValue and isFirstChange. + // `previousValue` and `isFirstChange`. const pendingChange = this.inputChanges[property]; if (pendingChange) { pendingChange.currentValue = currentValue; @@ -265,8 +288,6 @@ export class ComponentNgElementStrategy implements NgElementStrategy { } const isFirstChange = this.unchangedInputs.has(property); - this.unchangedInputs.delete(property); - const previousValue = isFirstChange ? undefined : this.getInputValue(property); this.inputChanges[property] = new SimpleChange(previousValue, currentValue, isFirstChange); } @@ -278,6 +299,7 @@ export class ComponentNgElementStrategy implements NgElementStrategy { } this.callNgOnChanges(this.componentRef); + this.markViewForCheck(this.viewChangeDetectorRef!); this.componentRef.changeDetectorRef.detectChanges(); } diff --git a/packages/elements/test/component-factory-strategy_spec.ts b/packages/elements/test/component-factory-strategy_spec.ts index d2ae5d2fa9..00bcb97a50 100644 --- a/packages/elements/test/component-factory-strategy_spec.ts +++ b/packages/elements/test/component-factory-strategy_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ApplicationRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, Injector, NgModuleRef, NgZone, SimpleChange, SimpleChanges, Type} from '@angular/core'; +import {ApplicationRef, ChangeDetectorRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, Injector, NgModuleRef, NgZone, SimpleChange, SimpleChanges, Type} from '@angular/core'; import {fakeAsync, tick} from '@angular/core/testing'; import {Subject} from 'rxjs'; @@ -180,8 +180,11 @@ describe('ComponentFactoryNgElementStrategy', () => { }); describe('when inputs change and is connected', () => { + let viewChangeDetectorRef: ChangeDetectorRef; + beforeEach(() => { strategy.connect(document.createElement('div')); + viewChangeDetectorRef = componentRef.injector.get(ChangeDetectorRef); }); it('should be set on the component instance', () => { @@ -209,6 +212,22 @@ describe('ComponentFactoryNgElementStrategy', () => { expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(2); })); + it('should not detect changes if the input is set to the same value', fakeAsync(() => { + (componentRef.changeDetectorRef.detectChanges as jasmine.Spy).calls.reset(); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(1); + + (componentRef.changeDetectorRef.detectChanges as jasmine.Spy).calls.reset(); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + expect(componentRef.changeDetectorRef.detectChanges).not.toHaveBeenCalled(); + })); + it('should call ngOnChanges', fakeAsync(() => { strategy.setInputValue('fooFoo', 'fooFoo-1'); tick(16); // scheduler waits 16ms if RAF is unavailable @@ -246,6 +265,22 @@ describe('ComponentFactoryNgElementStrategy', () => { }); })); + it('should not call ngOnChanges if the inout is set to the same value', fakeAsync(() => { + const ngOnChangesSpy = spyOn(componentRef.instance, 'ngOnChanges'); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + expect(ngOnChangesSpy).toHaveBeenCalledTimes(1); + + ngOnChangesSpy.calls.reset(); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + expect(ngOnChangesSpy).not.toHaveBeenCalled(); + })); + it('should not try to call ngOnChanges if not present on the component', fakeAsync(() => { const factory2 = new FakeComponentFactory(FakeComponentWithoutNgOnChanges); const strategy2 = new ComponentNgElementStrategy(factory2, injector); @@ -261,6 +296,71 @@ describe('ComponentFactoryNgElementStrategy', () => { // been thrown and `changeDetectorRef2.detectChanges()` would not have been called. expect(changeDetectorRef2.detectChanges).toHaveBeenCalledTimes(1); })); + + it('should mark the view for check', fakeAsync(() => { + expect(viewChangeDetectorRef.markForCheck).not.toHaveBeenCalled(); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + + expect(viewChangeDetectorRef.markForCheck).toHaveBeenCalledTimes(1); + })); + + it('should mark the view for check once for multiple input changes', fakeAsync(() => { + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + + expect(viewChangeDetectorRef.markForCheck).toHaveBeenCalledTimes(1); + })); + + it('should mark the view for check twice for changes in different rounds with previous values', + fakeAsync(() => { + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + + expect(viewChangeDetectorRef.markForCheck).toHaveBeenCalledTimes(1); + + strategy.setInputValue('fooFoo', 'fooFoo-2'); + strategy.setInputValue('barBar', 'barBar-2'); + tick(16); // scheduler waits 16ms if RAF is unavailable + + expect(viewChangeDetectorRef.markForCheck).toHaveBeenCalledTimes(2); + })); + + it('should mark the view for check even if ngOnChanges is not present on the component', + fakeAsync(() => { + const factory2 = new FakeComponentFactory(FakeComponentWithoutNgOnChanges); + const strategy2 = new ComponentNgElementStrategy(factory2, injector); + const viewChangeDetectorRef2 = factory2.componentRef.injector.get(ChangeDetectorRef); + + strategy2.connect(document.createElement('div')); + (viewChangeDetectorRef2.markForCheck as jasmine.Spy).calls.reset(); + + strategy2.setInputValue('fooFoo', 'fooFoo-1'); + expect(() => tick(16)).not.toThrow(); // scheduler waits 16ms if RAF is unavailable + + // If the strategy would have tried to call `component.ngOnChanges()`, an error would have + // been thrown and `viewChangeDetectorRef2.markForCheck()` would not have been called. + expect(viewChangeDetectorRef2.markForCheck).toHaveBeenCalledTimes(1); + })); + + it('should not mark the view for check if the input is set to the same value', fakeAsync(() => { + (viewChangeDetectorRef.markForCheck as jasmine.Spy).calls.reset(); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + expect(viewChangeDetectorRef.markForCheck).toHaveBeenCalledTimes(1); + + (viewChangeDetectorRef.markForCheck as jasmine.Spy).calls.reset(); + + strategy.setInputValue('fooFoo', 'fooFoo-1'); + strategy.setInputValue('barBar', 'barBar-1'); + tick(16); // scheduler waits 16ms if RAF is unavailable + expect(viewChangeDetectorRef.markForCheck).not.toHaveBeenCalled(); + })); }); describe('disconnect', () => { @@ -345,6 +445,9 @@ export class FakeComponentFactory> extends ComponentFactory< { changeDetectorRef: jasmine.createSpyObj('changeDetectorRef', ['detectChanges']), hostView: {}, + injector: jasmine.createSpyObj('injector', { + get: jasmine.createSpyObj('viewChangeDetectorRef', ['markForCheck']), + }), instance: new this.ComponentClass(), });