Skip to content

Commit

Permalink
fix(core): Support extending differs from root NgModule (#39981)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mhevery committed Dec 7, 2020
1 parent 775394c commit 5fc4508
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 35 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/aio-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 3153,
"main-es2015": 431696,
"main-es2015": 431137,
"polyfills-es2015": 52493
}
}
Expand Down
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 140921,
"main-es2015": 140333,
"polyfills-es2015": 36964
}
}
Expand Down
24 changes: 11 additions & 13 deletions packages/core/src/change_detection/differs/iterable_differs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,19 @@ export interface IterableDifferFactory {
create<V>(trackByFn?: TrackByFunction<V>): IterableDiffer<V>;
}

export function defaultIterableDiffersFactory() {
return new IterableDiffers([new DefaultIterableDifferFactory()]);
}

/**
* A repository of different iterable diffing strategies used by NgFor, NgClass, and others.
*
* @publicApi
*/
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
Expand Down Expand Up @@ -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()]]
Expand Down
21 changes: 10 additions & 11 deletions packages/core/src/change_detection/differs/keyvalue_differs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,19 @@ export interface KeyValueDifferFactory {
create<K, V>(): KeyValueDiffer<K, V>;
}

export function defaultKeyValueDiffersFactory() {
return new KeyValueDiffers([new DefaultKeyValueDifferFactory()]);
}

/**
* A repository of different Map diffing strategies used by NgClass, NgStyle, and others.
*
* @publicApi
*/
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.
Expand Down Expand Up @@ -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()]]
Expand Down
6 changes: 6 additions & 0 deletions packages/core/test/bundling/forms/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -854,9 +854,15 @@
{
"name": "defaultIterableDiffers"
},
{
"name": "defaultIterableDiffersFactory"
},
{
"name": "defaultKeyValueDiffers"
},
{
"name": "defaultKeyValueDiffersFactory"
},
{
"name": "defaultScheduler"
},
Expand Down
6 changes: 6 additions & 0 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1121,9 +1121,15 @@
{
"name": "defaultIterableDiffers"
},
{
"name": "defaultIterableDiffersFactory"
},
{
"name": "defaultKeyValueDiffers"
},
{
"name": "defaultKeyValueDiffersFactory"
},
{
"name": "defaultMalformedUriErrorHandler"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,9 @@
{
"name": "defaultErrorLogger"
},
{
"name": "defaultIterableDiffersFactory"
},
{
"name": "defaultScheduler"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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}]);
Expand All @@ -66,6 +59,32 @@ import {SpyIterableDifferFactory} from '../../spies';
factory2, factory1
]);
});

it('should support .extend in root NgModule', () => {
const DIFFER: IterableDiffer<any> = {} as any;
const log: string[] = [];
class MyIterableDifferFactory implements IterableDifferFactory {
supports(objects: any): boolean {
log.push('supports', objects);
return true;
}
create<V>(trackByFn?: TrackByFunction<V>): IterableDiffer<V> {
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']);
});
});
});
}
Original file line number Diff line number Diff line change
@@ -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<any, any> = {} as any;
const log: string[] = [];
class MyKeyValueDifferFactory implements KeyValueDifferFactory {
supports(objects: any): boolean {
log.push('supports', objects);
return true;
}
create<K, V>(): KeyValueDiffer<K, V> {
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']);
});
});

0 comments on commit 5fc4508

Please sign in to comment.