From f2828a08bd3d08fa8825b8aeafffde6dafe63444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Mon, 11 Nov 2019 15:43:09 -0800 Subject: [PATCH] fix(ivy): ExpressionChangedAfterItHasBeenCheckedError for SafeValue (#33749) Fix #33448 PR Close #33749 --- .../core/src/render3/instructions/styling.ts | 6 +++--- .../core/src/render3/util/styling_utils.ts | 9 +++++++++ packages/core/src/sanitization/bypass.ts | 8 +++++--- packages/core/test/acceptance/styling_spec.ts | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index f306c51c70..44862eb531 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -13,14 +13,14 @@ import {AttributeMarker, TAttributes, TNode, TNodeFlags, TNodeType} from '../int import {RElement} from '../interfaces/renderer'; import {StylingMapArray, StylingMapArrayIndex, TStylingContext} from '../interfaces/styling'; import {isDirectiveHost} from '../interfaces/type_checks'; -import {LView, RENDERER, TVIEW, TView} from '../interfaces/view'; +import {LView, RENDERER, TVIEW} from '../interfaces/view'; import {getActiveDirectiveId, getCheckNoChangesMode, getCurrentStyleSanitizer, getLView, getSelectedIndex, incrementBindingIndex, nextBindingIndex, resetCurrentStyleSanitizer, setCurrentStyleSanitizer, setElementExitFn} from '../state'; import {applyStylingMapDirectly, applyStylingValueDirectly, flushStyling, setClass, setStyle, updateClassViaContext, updateStyleViaContext} from '../styling/bindings'; import {activateStylingMapFeature} from '../styling/map_based_bindings'; import {attachStylingDebugObject} from '../styling/styling_debug'; import {NO_CHANGE} from '../tokens'; import {renderStringify} from '../util/misc_utils'; -import {addItemToStylingMap, allocStylingMapArray, allocTStylingContext, allowDirectStyling, concatString, forceClassesAsString, forceStylesAsString, getInitialStylingValue, getStylingMapArray, getValue, hasClassInput, hasStyleInput, hasValueChanged, isHostStylingActive, isStylingContext, isStylingValueDefined, normalizeIntoStylingMap, patchConfig, selectClassBasedInputName, setValue, stylingMapToString} from '../util/styling_utils'; +import {addItemToStylingMap, allocStylingMapArray, allocTStylingContext, allowDirectStyling, concatString, forceClassesAsString, forceStylesAsString, getInitialStylingValue, getStylingMapArray, getValue, hasClassInput, hasStyleInput, hasValueChanged, hasValueChangedUnwrapSafeValue, isHostStylingActive, isStylingContext, isStylingValueDefined, normalizeIntoStylingMap, patchConfig, selectClassBasedInputName, setValue, stylingMapToString} from '../util/styling_utils'; import {getNativeByTNode, getTNode} from '../util/view_utils'; @@ -187,7 +187,7 @@ function stylingProp( // as well. if (ngDevMode && getCheckNoChangesMode()) { const oldValue = getValue(lView, bindingIndex); - if (hasValueChanged(oldValue, value)) { + if (hasValueChangedUnwrapSafeValue(oldValue, value)) { throwErrorIfNoChangesMode(false, oldValue, value); } } diff --git a/packages/core/src/render3/util/styling_utils.ts b/packages/core/src/render3/util/styling_utils.ts index 6239eda7bd..c86a5c0171 100644 --- a/packages/core/src/render3/util/styling_utils.ts +++ b/packages/core/src/render3/util/styling_utils.ts @@ -5,6 +5,7 @@ * 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 {unwrapSafeValue} from '../../sanitization/bypass'; import {PropertyAliases, TNodeFlags} from '../interfaces/node'; import {LStylingData, StylingMapArray, StylingMapArrayIndex, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags, TStylingNode} from '../interfaces/styling'; import {NO_CHANGE} from '../tokens'; @@ -170,6 +171,14 @@ export function getPropValuesStartPosition( return startPosition; } +export function hasValueChangedUnwrapSafeValue( + a: NO_CHANGE | StylingMapArray | number | String | string | null | boolean | undefined | {}, + b: NO_CHANGE | StylingMapArray | number | String | string | null | boolean | undefined | + {}): boolean { + return hasValueChanged(unwrapSafeValue(a), unwrapSafeValue(b)); +} + + export function hasValueChanged( a: NO_CHANGE | StylingMapArray | number | string | null | boolean | undefined | {}, b: NO_CHANGE | StylingMapArray | number | string | null | boolean | undefined | {}): boolean { diff --git a/packages/core/src/sanitization/bypass.ts b/packages/core/src/sanitization/bypass.ts index 407f903d55..a72b20ada6 100644 --- a/packages/core/src/sanitization/bypass.ts +++ b/packages/core/src/sanitization/bypass.ts @@ -87,9 +87,11 @@ class SafeResourceUrlImpl extends SafeValueImpl implements SafeResourceUrl { getTypeName() { return BypassType.ResourceUrl; } } -export function unwrapSafeValue(value: string | SafeValue): string { - return value instanceof SafeValueImpl ? value.changingThisBreaksApplicationSecurity : - value as string; +export function unwrapSafeValue(value: SafeValue): string; +export function unwrapSafeValue(value: T): T; +export function unwrapSafeValue(value: T | SafeValue): T { + return value instanceof SafeValueImpl ? value.changingThisBreaksApplicationSecurity as any as T : + value as any as T; } diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index f2a8d44dc8..ec4b66b3e4 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -2365,6 +2365,25 @@ describe('styling', () => { function splitSortJoin(s: string) { return s.split(/\s+/).sort().join(' ').trim(); } }); + + describe('ExpressionChangedAfterItHasBeenCheckedError', () => { + it('should not throw when bound to SafeValue', () => { + @Component({template: `
`}) + class MyComp { + icon = 'https://i.imgur.com/4AiXzf8.jpg'; + get iconSafe() { return this.sanitizer.bypassSecurityTrustStyle(`url("${this.icon}")`); } + + constructor(private sanitizer: DomSanitizer) {} + } + + const fixture = + TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp); + fixture.detectChanges(true /* Verify that check no changes does not cause an exception */); + const div: HTMLElement = fixture.nativeElement.querySelector('div'); + expect(div.style.getPropertyValue('background-image')) + .toEqual('url("https://i.imgur.com/4AiXzf8.jpg")'); + }); + }); }); function assertStyleCounters(countForSet: number, countForRemove: number) {