Skip to content

Commit

Permalink
fix(core): input coercion (#42803)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Forms [email] input coercion

Forms [email] input value will be considered as true if it is defined with any value rather
than false and 'false'.

PR Close #42803
  • Loading branch information
lekhmanrus authored and dylhunn committed Feb 7, 2022
1 parent db6cf7e commit d5719c2
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 39 deletions.
1 change: 1 addition & 0 deletions packages/core/src/application_init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import {Observable} from 'rxjs';

import {Inject, Injectable, InjectionToken, Optional} from './di';
import {isObservable, isPromise} from './util/lang';
import {noop} from './util/noop';
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/change_detection/change_detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ export {SimpleChange, SimpleChanges} from '../interface/simple_change';
export {devModeEqual} from './change_detection_util';
export {ChangeDetectorRef} from './change_detector_ref';
export {ChangeDetectionStrategy, ChangeDetectorStatus, isDefaultChangeDetectionStrategy} from './constants';
export {DefaultIterableDifferFactory} from './differs/default_iterable_differ';
export {DefaultIterableDiffer} from './differs/default_iterable_differ';
export {DefaultIterableDiffer, DefaultIterableDifferFactory} from './differs/default_iterable_differ';
export {DefaultKeyValueDifferFactory} from './differs/default_keyvalue_differ';
export {IterableChangeRecord, IterableChanges, IterableDiffer, IterableDifferFactory, IterableDiffers, NgIterable, TrackByFunction} from './differs/iterable_differs';
export {KeyValueChangeRecord, KeyValueChanges, KeyValueDiffer, KeyValueDifferFactory, KeyValueDiffers} from './differs/keyvalue_differs';
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export {GetterFn as ɵGetterFn, MethodFn as ɵMethodFn, SetterFn as ɵSetterFn}
export {allowSanitizationBypassAndThrow as ɵallowSanitizationBypassAndThrow, BypassType as ɵBypassType, getSanitizationBypassType as ɵgetSanitizationBypassType, SafeHtml as ɵSafeHtml, SafeResourceUrl as ɵSafeResourceUrl, SafeScript as ɵSafeScript, SafeStyle as ɵSafeStyle, SafeUrl as ɵSafeUrl, SafeValue as ɵSafeValue, unwrapSafeValue as ɵunwrapSafeValue} from './sanitization/bypass';
export {_sanitizeHtml as ɵ_sanitizeHtml} from './sanitization/html_sanitizer';
export {_sanitizeUrl as ɵ_sanitizeUrl} from './sanitization/url_sanitizer';
export {coerceToBoolean as ɵcoerceToBoolean} from './util/coercion';
export {makeDecorator as ɵmakeDecorator} from './util/decorators';
export {global as ɵglobal} from './util/global';
export {isObservable as ɵisObservable, isPromise as ɵisPromise, isSubscribable as ɵisSubscribable} from './util/lang';
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/reflection/reflector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
*/

import {Type} from '../interface/type';

import {PlatformReflectionCapabilities} from './platform_reflection_capabilities';
import {GetterFn, MethodFn, SetterFn} from './types';

export {PlatformReflectionCapabilities};
export {GetterFn, MethodFn, SetterFn};
export {GetterFn, MethodFn, PlatformReflectionCapabilities, SetterFn};

/**
* Provides access to reflection data about symbols. Used internally by Angular
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/context_discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import '../util/ng_dev_mode';

import {assertDefined, assertDomNode} from '../util/assert';

import {EMPTY_ARRAY} from '../util/empty';

import {LContext} from './interfaces/context';
import {TNode, TNodeFlags} from './interfaces/node';
import {RElement, RNode} from './interfaces/renderer_dom';
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/util/coercion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* @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
*/

/** Coerces a value (typically a string) to a boolean. */
export function coerceToBoolean(value: unknown): boolean {
return typeof value === 'boolean' ? value : (value != null && value !== 'false');
}
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,9 @@
{
"name": "coerceToAsyncValidator"
},
{
"name": "coerceToBoolean"
},
{
"name": "coerceToValidator"
},
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -878,9 +878,6 @@
{
"name": "attachPatchData"
},
{
"name": "attrBoolValue"
},
{
"name": "autoRegisterModuleById"
},
Expand Down Expand Up @@ -911,6 +908,9 @@
{
"name": "cleanUpView"
},
{
"name": "coerceToBoolean"
},
{
"name": "collectNativeNodes"
},
Expand Down
56 changes: 56 additions & 0 deletions packages/core/test/util/coercion_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* @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 {coerceToBoolean} from '@angular/core/src/util/coercion';

{
describe('coerceToBoolean', () => {
it('should coerce undefined to false', () => {
expect(coerceToBoolean(undefined)).toBe(false);
});

it('should coerce null to false', () => {
expect(coerceToBoolean(null)).toBe(false);
});

it('should coerce the empty string to true', () => {
expect(coerceToBoolean('')).toBe(true);
});

it('should coerce zero to true', () => {
expect(coerceToBoolean(0)).toBe(true);
});

it('should coerce the string "false" to false', () => {
expect(coerceToBoolean('false')).toBe(false);
});

it('should coerce the boolean false to false', () => {
expect(coerceToBoolean(false)).toBe(false);
});

it('should coerce the boolean true to true', () => {
expect(coerceToBoolean(true)).toBe(true);
});

it('should coerce the string "true" to true', () => {
expect(coerceToBoolean('true')).toBe(true);
});

it('should coerce an arbitrary string to true', () => {
expect(coerceToBoolean('pink')).toBe(true);
});

it('should coerce an object to true', () => {
expect(coerceToBoolean({})).toBe(true);
});

it('should coerce an array to true', () => {
expect(coerceToBoolean([])).toBe(true);
});
});
}
6 changes: 3 additions & 3 deletions packages/forms/src/directives/ng_model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ChangeDetectorRef, Directive, EventEmitter, forwardRef, Host, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges} from '@angular/core';
import {ChangeDetectorRef, Directive, EventEmitter, forwardRef, Host, Inject, Input, OnChanges, OnDestroy, Optional, Output, Self, SimpleChanges, ɵcoerceToBoolean as coerceToBoolean} from '@angular/core';

import {FormControl, FormHooks} from '../model';
import {NG_ASYNC_VALIDATORS, NG_VALIDATORS} from '../validators';
Expand Down Expand Up @@ -320,8 +320,8 @@ export class NgModel extends NgControl implements OnChanges, OnDestroy {

private _updateDisabled(changes: SimpleChanges) {
const disabledValue = changes['isDisabled'].currentValue;

const isDisabled = disabledValue === '' || (disabledValue && disabledValue !== 'false');
// checking for 0 to avoid breaking change
const isDisabled = disabledValue !== 0 && coerceToBoolean(disabledValue);

resolvedPromise.then(() => {
if (isDisabled && !this.control.disabled) {
Expand Down
20 changes: 3 additions & 17 deletions packages/forms/src/directives/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, forwardRef, Input, OnChanges, SimpleChanges, StaticProvider} from '@angular/core';
import {Directive, forwardRef, Input, OnChanges, SimpleChanges, StaticProvider, ɵcoerceToBoolean as coerceToBoolean} from '@angular/core';
import {Observable} from 'rxjs';

import {AbstractControl} from '../model';
Expand All @@ -22,16 +22,6 @@ function toInteger(value: string|number): number {
return typeof value === 'number' ? value : parseInt(value, 10);
}

/**
* Method that converts null, false or 'false' string to boolean.
*
* @param value input value.
* @returns value of parameter converted to boolean.
*/
function toBoolean(input: unknown): boolean {
return input != null && input !== false && `${input}` !== 'false';
}

/**
* Method that ensures that provided value is a float (and converts it to float if needed).
*
Expand Down Expand Up @@ -380,7 +370,7 @@ export class RequiredValidator extends AbstractValidatorDirective {
override inputName = 'required';

/** @internal */
override normalizeInput = (input: unknown): boolean => toBoolean(input);
override normalizeInput = coerceToBoolean;

/** @internal */
override createValidator = (input: boolean): ValidatorFn => requiredValidator;
Expand Down Expand Up @@ -472,11 +462,7 @@ export class EmailValidator extends AbstractValidatorDirective {
override inputName = 'email';

/** @internal */
override normalizeInput = (input: unknown): boolean =>
// Avoid TSLint requirement to omit semicolon, see
// https://github.com/palantir/tslint/issues/1476
// tslint:disable-next-line:semicolon
(input === '' || input === true || input === 'true');
override normalizeInput = coerceToBoolean;

/** @internal */
override createValidator = (input: number): ValidatorFn => emailValidator;
Expand Down
18 changes: 7 additions & 11 deletions packages/router/src/directives/router_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {LocationStrategy} from '@angular/common';
import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, OnChanges, OnDestroy, Renderer2, SimpleChanges} from '@angular/core';
import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, OnChanges, OnDestroy, Renderer2, SimpleChanges, ɵcoerceToBoolean as coerceToBoolean} from '@angular/core';
import {Subject, Subscription} from 'rxjs';

import {Event, NavigationEnd} from '../events';
Expand Down Expand Up @@ -242,8 +242,8 @@ export class RouterLink implements OnChanges {
}

const extras = {
skipLocationChange: attrBoolValue(this.skipLocationChange),
replaceUrl: attrBoolValue(this.replaceUrl),
skipLocationChange: coerceToBoolean(this.skipLocationChange),
replaceUrl: coerceToBoolean(this.replaceUrl),
state: this.state,
};
this.router.navigateByUrl(this.urlTree, extras);
Expand All @@ -261,7 +261,7 @@ export class RouterLink implements OnChanges {
queryParams: this.queryParams,
fragment: this.fragment,
queryParamsHandling: this.queryParamsHandling,
preserveFragment: attrBoolValue(this.preserveFragment),
preserveFragment: coerceToBoolean(this.preserveFragment),
});
}
}
Expand Down Expand Up @@ -406,8 +406,8 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
}

const extras = {
skipLocationChange: attrBoolValue(this.skipLocationChange),
replaceUrl: attrBoolValue(this.replaceUrl),
skipLocationChange: coerceToBoolean(this.skipLocationChange),
replaceUrl: coerceToBoolean(this.replaceUrl),
state: this.state
};
this.router.navigateByUrl(this.urlTree, extras);
Expand All @@ -431,11 +431,7 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
queryParams: this.queryParams,
fragment: this.fragment,
queryParamsHandling: this.queryParamsHandling,
preserveFragment: attrBoolValue(this.preserveFragment),
preserveFragment: coerceToBoolean(this.preserveFragment),
});
}
}

function attrBoolValue(s: any): boolean {
return s === '' || !!s;
}

0 comments on commit d5719c2

Please sign in to comment.