From 5fc45082cae33e8ce95d95560ba67e6388d07732 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 4 Dec 2020 16:39:37 -0800 Subject: [PATCH] fix(core): Support extending differs from root `NgModule` (#39981) Differs tries to inject parent differ in order to support extending. This does not work in the 'root' injector as the provider overrides the default injector. The fix is to just assume standard set of providers and extend those instead. PR close #25015 Issue close #11309 `Can't extend IterableDiffers` Issue close #18554 `IterableDiffers.extend is not AOT compatible` (This is fixed because we no longer have an arrow function in the factory but a proper function which can be imported.) PR Close #39981 --- goldens/size-tracking/aio-payloads.json | 2 +- .../size-tracking/integration-payloads.json | 2 +- .../differs/iterable_differs.ts | 24 ++++++------ .../differs/keyvalue_differs.ts | 21 +++++----- .../bundling/forms/bundle.golden_symbols.json | 6 +++ .../router/bundle.golden_symbols.json | 6 +++ .../bundling/todo/bundle.golden_symbols.json | 3 ++ .../differs/iterable_differs_spec.ts | 37 +++++++++++++----- .../differs/keyvalue_differs_spec.ts | 39 +++++++++++++++++++ 9 files changed, 105 insertions(+), 35 deletions(-) create mode 100644 packages/core/test/change_detection/differs/keyvalue_differs_spec.ts diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index 0f3cb254a5..1538d4b8f8 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime-es2015": 3153, - "main-es2015": 431696, + "main-es2015": 431137, "polyfills-es2015": 52493 } } diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 9022471479..9d14be1e8b 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -3,7 +3,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 140921, + "main-es2015": 140333, "polyfills-es2015": 36964 } } diff --git a/packages/core/src/change_detection/differs/iterable_differs.ts b/packages/core/src/change_detection/differs/iterable_differs.ts index b9192746be..69c2d80fb6 100644 --- a/packages/core/src/change_detection/differs/iterable_differs.ts +++ b/packages/core/src/change_detection/differs/iterable_differs.ts @@ -134,6 +134,10 @@ export interface IterableDifferFactory { create(trackByFn?: TrackByFunction): IterableDiffer; } +export function defaultIterableDiffersFactory() { + return new IterableDiffers([new DefaultIterableDifferFactory()]); +} + /** * A repository of different iterable diffing strategies used by NgFor, NgClass, and others. * @@ -141,11 +145,8 @@ export interface IterableDifferFactory { */ export class IterableDiffers { /** @nocollapse */ - static ɵprov = ɵɵdefineInjectable({ - token: IterableDiffers, - providedIn: 'root', - factory: () => new IterableDiffers([new DefaultIterableDifferFactory()]) - }); + static ɵprov = ɵɵdefineInjectable( + {token: IterableDiffers, providedIn: 'root', factory: defaultIterableDiffersFactory}); /** * @deprecated v4.0.0 - Should be private @@ -187,14 +188,11 @@ export class IterableDiffers { static extend(factories: IterableDifferFactory[]): StaticProvider { return { provide: IterableDiffers, - useFactory: (parent: IterableDiffers) => { - if (!parent) { - // Typically would occur when calling IterableDiffers.extend inside of dependencies passed - // to - // bootstrap(), which would override default pipes instead of extending them. - throw new Error('Cannot extend IterableDiffers without a parent injector'); - } - return IterableDiffers.create(factories, parent); + useFactory: (parent: IterableDiffers|null) => { + // if parent is null, it means that we are in the root injector and we have just overridden + // the default injection mechanism for IterableDiffers, in such a case just assume + // `defaultIterableDiffersFactory`. + return IterableDiffers.create(factories, parent || defaultIterableDiffersFactory()); }, // Dependency technically isn't optional, but we can provide a better error message this way. deps: [[IterableDiffers, new SkipSelf(), new Optional()]] diff --git a/packages/core/src/change_detection/differs/keyvalue_differs.ts b/packages/core/src/change_detection/differs/keyvalue_differs.ts index 7f9bd89a3d..ac31aa0aab 100644 --- a/packages/core/src/change_detection/differs/keyvalue_differs.ts +++ b/packages/core/src/change_detection/differs/keyvalue_differs.ts @@ -111,6 +111,10 @@ export interface KeyValueDifferFactory { create(): KeyValueDiffer; } +export function defaultKeyValueDiffersFactory() { + return new KeyValueDiffers([new DefaultKeyValueDifferFactory()]); +} + /** * A repository of different Map diffing strategies used by NgClass, NgStyle, and others. * @@ -118,11 +122,8 @@ export interface KeyValueDifferFactory { */ export class KeyValueDiffers { /** @nocollapse */ - static ɵprov = ɵɵdefineInjectable({ - token: KeyValueDiffers, - providedIn: 'root', - factory: () => new KeyValueDiffers([new DefaultKeyValueDifferFactory()]) - }); + static ɵprov = ɵɵdefineInjectable( + {token: KeyValueDiffers, providedIn: 'root', factory: defaultKeyValueDiffersFactory}); /** * @deprecated v4.0.0 - Should be private. @@ -165,12 +166,10 @@ export class KeyValueDiffers { return { provide: KeyValueDiffers, useFactory: (parent: KeyValueDiffers) => { - if (!parent) { - // Typically would occur when calling KeyValueDiffers.extend inside of dependencies passed - // to bootstrap(), which would override default pipes instead of extending them. - throw new Error('Cannot extend KeyValueDiffers without a parent injector'); - } - return KeyValueDiffers.create(factories, parent); + // if parent is null, it means that we are in the root injector and we have just overridden + // the default injection mechanism for KeyValueDiffers, in such a case just assume + // `defaultKeyValueDiffersFactory`. + return KeyValueDiffers.create(factories, parent || defaultKeyValueDiffersFactory()); }, // Dependency technically isn't optional, but we can provide a better error message this way. deps: [[KeyValueDiffers, new SkipSelf(), new Optional()]] diff --git a/packages/core/test/bundling/forms/bundle.golden_symbols.json b/packages/core/test/bundling/forms/bundle.golden_symbols.json index 8535a95744..bf44a9ff83 100644 --- a/packages/core/test/bundling/forms/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms/bundle.golden_symbols.json @@ -854,9 +854,15 @@ { "name": "defaultIterableDiffers" }, + { + "name": "defaultIterableDiffersFactory" + }, { "name": "defaultKeyValueDiffers" }, + { + "name": "defaultKeyValueDiffersFactory" + }, { "name": "defaultScheduler" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 036c745dba..a6c8ad6d13 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1121,9 +1121,15 @@ { "name": "defaultIterableDiffers" }, + { + "name": "defaultIterableDiffersFactory" + }, { "name": "defaultKeyValueDiffers" }, + { + "name": "defaultKeyValueDiffersFactory" + }, { "name": "defaultMalformedUriErrorHandler" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 2162f45566..839e1f4e85 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -281,6 +281,9 @@ { "name": "defaultErrorLogger" }, + { + "name": "defaultIterableDiffersFactory" + }, { "name": "defaultScheduler" }, diff --git a/packages/core/test/change_detection/differs/iterable_differs_spec.ts b/packages/core/test/change_detection/differs/iterable_differs_spec.ts index 5563a85d51..19cdd0e691 100644 --- a/packages/core/test/change_detection/differs/iterable_differs_spec.ts +++ b/packages/core/test/change_detection/differs/iterable_differs_spec.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {Injector} from '@angular/core'; -import {IterableDiffers} from '@angular/core/src/change_detection/differs/iterable_differs'; +import {Injector, IterableDiffer, IterableDifferFactory, IterableDiffers, NgModule, TrackByFunction} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; import {SpyIterableDifferFactory} from '../../spies'; @@ -49,13 +49,6 @@ import {SpyIterableDifferFactory} from '../../spies'; }); describe('.extend()', () => { - it('should throw if calling extend when creating root injector', () => { - const injector = Injector.create([IterableDiffers.extend([])]); - - expect(() => injector.get(IterableDiffers)) - .toThrowError(/Cannot extend IterableDiffers without a parent injector/); - }); - it('should extend di-inherited differs', () => { const parent = new IterableDiffers([factory1]); const injector = Injector.create([{provide: IterableDiffers, useValue: parent}]); @@ -66,6 +59,32 @@ import {SpyIterableDifferFactory} from '../../spies'; factory2, factory1 ]); }); + + it('should support .extend in root NgModule', () => { + const DIFFER: IterableDiffer = {} as any; + const log: string[] = []; + class MyIterableDifferFactory implements IterableDifferFactory { + supports(objects: any): boolean { + log.push('supports', objects); + return true; + } + create(trackByFn?: TrackByFunction): IterableDiffer { + log.push('create'); + return DIFFER; + } + } + + + @NgModule({providers: [IterableDiffers.extend([new MyIterableDifferFactory()])]}) + class MyModule { + } + + TestBed.configureTestingModule({imports: [MyModule]}); + const differs = TestBed.inject(IterableDiffers); + const differ = differs.find('VALUE').create(null!); + expect(differ).toEqual(DIFFER); + expect(log).toEqual(['supports', 'VALUE', 'create']); + }); }); }); } diff --git a/packages/core/test/change_detection/differs/keyvalue_differs_spec.ts b/packages/core/test/change_detection/differs/keyvalue_differs_spec.ts new file mode 100644 index 0000000000..4e96b3ad0e --- /dev/null +++ b/packages/core/test/change_detection/differs/keyvalue_differs_spec.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {KeyValueDiffer, KeyValueDifferFactory, KeyValueDiffers, NgModule} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; + + +describe('KeyValueDiffers', function() { + it('should support .extend in root NgModule', () => { + const DIFFER: KeyValueDiffer = {} as any; + const log: string[] = []; + class MyKeyValueDifferFactory implements KeyValueDifferFactory { + supports(objects: any): boolean { + log.push('supports', objects); + return true; + } + create(): KeyValueDiffer { + log.push('create'); + return DIFFER; + } + } + + + @NgModule({providers: [KeyValueDiffers.extend([new MyKeyValueDifferFactory()])]}) + class MyModule { + } + + TestBed.configureTestingModule({imports: [MyModule]}); + const differs = TestBed.inject(KeyValueDiffers); + const differ = differs.find('VALUE').create(); + expect(differ).toEqual(DIFFER); + expect(log).toEqual(['supports', 'VALUE', 'create']); + }); +});