Skip to content

Commit

Permalink
perf(forms): use ngDevMode to tree-shake _ngModelWarning (angular…
Browse files Browse the repository at this point in the history
…#39964)

This commit adds `ngDevMode` guard to call `_ngModelWarning` only
in dev mode (similar to how things work in other parts of Ivy runtime code).
The `ngDevMode` flag helps to tree-shake this function from production builds
(since it will act as no-op, in dev mode everything will work as it works right now)
to decrease production bundle size.

PR Close angular#39964
  • Loading branch information
arturovt authored and mhevery committed Dec 5, 2020
1 parent 9ebe423 commit 5b47085
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 42 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
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
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
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
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
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
Expand Up @@ -281,6 +281,9 @@
{
"name": "defaultErrorLogger"
},
{
"name": "defaultIterableDiffersFactory"
},
{
"name": "defaultScheduler"
},
Expand Down
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']);
});
});
});
}
@@ -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']);
});
});
Expand Up @@ -125,7 +125,9 @@ export class FormControlDirective extends NgControl implements OnChanges {
this.form.updateValueAndValidity({emitEvent: false});
}
if (isPropertyUpdated(changes, this.viewModel)) {
_ngModelWarning('formControl', FormControlDirective, this, this._ngModelWarningConfig);
if (typeof ngDevMode === 'undefined' || ngDevMode) {
_ngModelWarning('formControl', FormControlDirective, this, this._ngModelWarningConfig);
}
this.form.setValue(this.model);
this.viewModel = this.model;
}
Expand Down
Expand Up @@ -145,7 +145,9 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy {
ngOnChanges(changes: SimpleChanges) {
if (!this._added) this._setUpControl();
if (isPropertyUpdated(changes, this.viewModel)) {
_ngModelWarning('formControlName', FormControlName, this, this._ngModelWarningConfig);
if (typeof ngDevMode === 'undefined' || ngDevMode) {
_ngModelWarning('formControlName', FormControlName, this, this._ngModelWarningConfig);
}
this.viewModel = this.model;
this.formDirective.updateModel(this, this.model);
}
Expand Down
8 changes: 2 additions & 6 deletions packages/forms/src/directives/shared.ts
Expand Up @@ -6,8 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import {isDevMode} from '@angular/core';

import {AbstractControl, FormArray, FormControl, FormGroup} from '../model';
import {getControlAsyncValidators, getControlValidators, mergeValidators} from '../validators';

Expand Down Expand Up @@ -324,13 +322,11 @@ export function removeListItem<T>(list: T[], el: T): void {
export function _ngModelWarning(
name: string, type: {_ngModelWarningSentOnce: boolean},
instance: {_ngModelWarningSent: boolean}, warningConfig: string|null) {
if (!isDevMode() || warningConfig === 'never') return;
if (warningConfig === 'never') return;

if (((warningConfig === null || warningConfig === 'once') && !type._ngModelWarningSentOnce) ||
(warningConfig === 'always' && !instance._ngModelWarningSent)) {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
ReactiveErrors.ngModelWarning(name);
}
ReactiveErrors.ngModelWarning(name);
type._ngModelWarningSentOnce = true;
instance._ngModelWarningSent = true;
}
Expand Down

0 comments on commit 5b47085

Please sign in to comment.