From d5719c2e0fb237be71d658444bbfe2410e05086a Mon Sep 17 00:00:00 2001 From: Ruslan Lekhman Date: Thu, 8 Jul 2021 19:04:30 -0600 Subject: [PATCH] fix(core): input coercion (#42803) 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 --- packages/core/src/application_init.ts | 1 + .../src/change_detection/change_detection.ts | 3 +- packages/core/src/core_private_export.ts | 1 + packages/core/src/reflection/reflector.ts | 4 +- .../core/src/render3/context_discovery.ts | 2 +- packages/core/src/util/coercion.ts | 12 ++++ .../bundle.golden_symbols.json | 3 + .../router/bundle.golden_symbols.json | 6 +- packages/core/test/util/coercion_spec.ts | 56 +++++++++++++++++++ packages/forms/src/directives/ng_model.ts | 6 +- packages/forms/src/directives/validators.ts | 20 +------ packages/router/src/directives/router_link.ts | 18 +++--- 12 files changed, 93 insertions(+), 39 deletions(-) create mode 100644 packages/core/src/util/coercion.ts create mode 100644 packages/core/test/util/coercion_spec.ts diff --git a/packages/core/src/application_init.ts b/packages/core/src/application_init.ts index b9a46d6f75f..8352b2fc81f 100644 --- a/packages/core/src/application_init.ts +++ b/packages/core/src/application_init.ts @@ -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'; diff --git a/packages/core/src/change_detection/change_detection.ts b/packages/core/src/change_detection/change_detection.ts index 59afd98ae70..4573e7f6006 100644 --- a/packages/core/src/change_detection/change_detection.ts +++ b/packages/core/src/change_detection/change_detection.ts @@ -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'; diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index 52500949b64..28456db4291 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -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'; diff --git a/packages/core/src/reflection/reflector.ts b/packages/core/src/reflection/reflector.ts index a8e2b156433..4f1292a667d 100644 --- a/packages/core/src/reflection/reflector.ts +++ b/packages/core/src/reflection/reflector.ts @@ -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 diff --git a/packages/core/src/render3/context_discovery.ts b/packages/core/src/render3/context_discovery.ts index 43f4207eb7a..ebc76422666 100644 --- a/packages/core/src/render3/context_discovery.ts +++ b/packages/core/src/render3/context_discovery.ts @@ -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'; diff --git a/packages/core/src/util/coercion.ts b/packages/core/src/util/coercion.ts new file mode 100644 index 00000000000..6ed07379251 --- /dev/null +++ b/packages/core/src/util/coercion.ts @@ -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'); +} diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 193bb0cdaba..f41f7ef9744 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -668,6 +668,9 @@ { "name": "coerceToAsyncValidator" }, + { + "name": "coerceToBoolean" + }, { "name": "coerceToValidator" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index b7f19737d9b..7ec67915e63 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -878,9 +878,6 @@ { "name": "attachPatchData" }, - { - "name": "attrBoolValue" - }, { "name": "autoRegisterModuleById" }, @@ -911,6 +908,9 @@ { "name": "cleanUpView" }, + { + "name": "coerceToBoolean" + }, { "name": "collectNativeNodes" }, diff --git a/packages/core/test/util/coercion_spec.ts b/packages/core/test/util/coercion_spec.ts new file mode 100644 index 00000000000..97a7a8aabb6 --- /dev/null +++ b/packages/core/test/util/coercion_spec.ts @@ -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); + }); + }); +} diff --git a/packages/forms/src/directives/ng_model.ts b/packages/forms/src/directives/ng_model.ts index 88fce139b21..8c618a67506 100644 --- a/packages/forms/src/directives/ng_model.ts +++ b/packages/forms/src/directives/ng_model.ts @@ -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'; @@ -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) { diff --git a/packages/forms/src/directives/validators.ts b/packages/forms/src/directives/validators.ts index 40634492c6c..9181775ca5c 100644 --- a/packages/forms/src/directives/validators.ts +++ b/packages/forms/src/directives/validators.ts @@ -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'; @@ -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). * @@ -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; @@ -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; diff --git a/packages/router/src/directives/router_link.ts b/packages/router/src/directives/router_link.ts index 8b019807ab4..4b0d9bd48a4 100644 --- a/packages/router/src/directives/router_link.ts +++ b/packages/router/src/directives/router_link.ts @@ -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'; @@ -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); @@ -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), }); } } @@ -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); @@ -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; -}