From c3f14bb7a55ff7a15be4f7a4fa7daa47321468cc Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Thu, 19 Dec 2019 17:02:51 -0800 Subject: [PATCH] feat(ivy): improve `ExpressionChangedAfterChecked` error message for attributes (#34505) This commit improves `ExpressionChangedAfterChecked` error message for attributes by including attribute name and the content of the entire expression that contains interpolation(s). In order to achieve that, metadata is now stored in `TData` array when `attribute` and `attributeInterpolate` instructions are being called (similar to `property` and `propertyInterpolate` instructions). PR Close #34505 --- .../src/render3/instructions/attribute.ts | 11 ++- .../instructions/attribute_interpolation.ts | 74 ++++++++++++++----- .../instructions/property_interpolation.ts | 8 +- .../test/acceptance/change_detection_spec.ts | 9 +-- 4 files changed, 70 insertions(+), 32 deletions(-) diff --git a/packages/core/src/render3/instructions/attribute.ts b/packages/core/src/render3/instructions/attribute.ts index a16876f390..b3d0442f27 100644 --- a/packages/core/src/render3/instructions/attribute.ts +++ b/packages/core/src/render3/instructions/attribute.ts @@ -7,9 +7,10 @@ */ import {bindingUpdated} from '../bindings'; import {SanitizerFn} from '../interfaces/sanitization'; +import {TVIEW} from '../interfaces/view'; import {getLView, getSelectedIndex, nextBindingIndex} from '../state'; -import {elementAttributeInternal} from './shared'; +import {elementAttributeInternal, storePropertyBindingMetadata} from './shared'; @@ -30,8 +31,12 @@ export function ɵɵattribute( name: string, value: any, sanitizer?: SanitizerFn | null, namespace?: string): typeof ɵɵattribute { const lView = getLView(); - if (bindingUpdated(lView, nextBindingIndex(), value)) { - elementAttributeInternal(getSelectedIndex(), name, value, lView, sanitizer, namespace); + const bindingIndex = nextBindingIndex(); + if (bindingUpdated(lView, bindingIndex, value)) { + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, name, value, lView, sanitizer, namespace); + ngDevMode && + storePropertyBindingMetadata(lView[TVIEW].data, nodeIndex, 'attr.' + name, bindingIndex); } return ɵɵattribute; } diff --git a/packages/core/src/render3/instructions/attribute_interpolation.ts b/packages/core/src/render3/instructions/attribute_interpolation.ts index ebf1f68f09..34ea69f205 100644 --- a/packages/core/src/render3/instructions/attribute_interpolation.ts +++ b/packages/core/src/render3/instructions/attribute_interpolation.ts @@ -6,11 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ import {SanitizerFn} from '../interfaces/sanitization'; -import {getLView, getSelectedIndex} from '../state'; +import {TVIEW} from '../interfaces/view'; +import {getBindingIndex, getLView, getSelectedIndex} from '../state'; import {NO_CHANGE} from '../tokens'; import {interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV} from './interpolation'; -import {elementAttributeInternal} from './shared'; +import {elementAttributeInternal, storePropertyBindingMetadata} from './shared'; @@ -44,8 +45,11 @@ export function ɵɵattributeInterpolate1( const lView = getLView(); const interpolatedValue = interpolation1(lView, prefix, v0, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 1, + prefix, suffix); } return ɵɵattributeInterpolate1; } @@ -82,8 +86,11 @@ export function ɵɵattributeInterpolate2( const lView = getLView(); const interpolatedValue = interpolation2(lView, prefix, v0, i0, v1, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 2, + prefix, i0, suffix); } return ɵɵattributeInterpolate2; } @@ -123,8 +130,11 @@ export function ɵɵattributeInterpolate3( const lView = getLView(); const interpolatedValue = interpolation3(lView, prefix, v0, i0, v1, i1, v2, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 3, + prefix, i0, i1, suffix); } return ɵɵattributeInterpolate3; } @@ -167,8 +177,11 @@ export function ɵɵattributeInterpolate4( const lView = getLView(); const interpolatedValue = interpolation4(lView, prefix, v0, i0, v1, i1, v2, i2, v3, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 4, + prefix, i0, i1, i2, suffix); } return ɵɵattributeInterpolate4; } @@ -214,8 +227,11 @@ export function ɵɵattributeInterpolate5( const interpolatedValue = interpolation5(lView, prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 5, + prefix, i0, i1, i2, i3, suffix); } return ɵɵattributeInterpolate5; } @@ -263,8 +279,11 @@ export function ɵɵattributeInterpolate6( const interpolatedValue = interpolation6(lView, prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 6, + prefix, i0, i1, i2, i3, i4, suffix); } return ɵɵattributeInterpolate6; } @@ -310,12 +329,15 @@ export function ɵɵattributeInterpolate7( attrName: string, prefix: string, v0: any, i0: string, v1: any, i1: string, v2: any, i2: string, v3: any, i3: string, v4: any, i4: string, v5: any, i5: string, v6: any, suffix: string, sanitizer?: SanitizerFn, namespace?: string): typeof ɵɵattributeInterpolate7 { - const index = getSelectedIndex(); const lView = getLView(); const interpolatedValue = interpolation7(lView, prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, i5, v6, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal(index, attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 7, + prefix, i0, i1, i2, i3, i4, i5, suffix); } return ɵɵattributeInterpolate7; } @@ -367,8 +389,11 @@ export function ɵɵattributeInterpolate8( const interpolatedValue = interpolation8( lView, prefix, v0, i0, v1, i1, v2, i2, v3, i3, v4, i4, v5, i5, v6, i6, v7, suffix); if (interpolatedValue !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolatedValue, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolatedValue, lView, sanitizer, namespace); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, getBindingIndex() - 8, + prefix, i0, i1, i2, i3, i4, i5, i6, suffix); } return ɵɵattributeInterpolate8; } @@ -405,8 +430,17 @@ export function ɵɵattributeInterpolateV( const lView = getLView(); const interpolated = interpolationV(lView, values); if (interpolated !== NO_CHANGE) { - elementAttributeInternal( - getSelectedIndex(), attrName, interpolated, lView, sanitizer, namespace); + const nodeIndex = getSelectedIndex(); + elementAttributeInternal(nodeIndex, attrName, interpolated, lView, sanitizer, namespace); + if (ngDevMode) { + const interpolationInBetween = [values[0]]; // prefix + for (let i = 2; i < values.length; i += 2) { + interpolationInBetween.push(values[i]); + } + storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, 'attr.' + attrName, + getBindingIndex() - interpolationInBetween.length + 1, ...interpolationInBetween); + } } return ɵɵattributeInterpolateV; } diff --git a/packages/core/src/render3/instructions/property_interpolation.ts b/packages/core/src/render3/instructions/property_interpolation.ts index 26949965ab..213d320e44 100644 --- a/packages/core/src/render3/instructions/property_interpolation.ts +++ b/packages/core/src/render3/instructions/property_interpolation.ts @@ -85,10 +85,10 @@ export function ɵɵpropertyInterpolate1( const lView = getLView(); const interpolatedValue = interpolation1(lView, prefix, v0, suffix); if (interpolatedValue !== NO_CHANGE) { - elementPropertyInternal(lView, getSelectedIndex(), propName, interpolatedValue, sanitizer); - ngDevMode && - storePropertyBindingMetadata( - lView[TVIEW].data, getSelectedIndex(), propName, getBindingIndex() - 1, prefix, suffix); + const nodeIndex = getSelectedIndex(); + elementPropertyInternal(lView, nodeIndex, propName, interpolatedValue, sanitizer); + ngDevMode && storePropertyBindingMetadata( + lView[TVIEW].data, nodeIndex, propName, getBindingIndex() - 1, prefix, suffix); } return ɵɵpropertyInterpolate1; } diff --git a/packages/core/test/acceptance/change_detection_spec.ts b/packages/core/test/acceptance/change_detection_spec.ts index bed117e249..10da2521b4 100644 --- a/packages/core/test/acceptance/change_detection_spec.ts +++ b/packages/core/test/acceptance/change_detection_spec.ts @@ -1301,17 +1301,16 @@ describe('change detection', () => { }); it('should include field name in case of attribute binding', () => { - // TODO(akushnir): improve error message and include attr name in Ivy - const message = ivyEnabled ? `Previous value: 'initial'. Current value: 'changed'` : - `Previous value: 'id: initial'. Current value: 'id: changed'`; + const message = ivyEnabled ? + `Previous value for 'attr.id': 'initial'. Current value: 'changed'` : + `Previous value: 'id: initial'. Current value: 'id: changed'`; expect(() => initWithTemplate('
')) .toThrowError(new RegExp(message)); }); it('should include field name in case of attribute interpolation', () => { - // TODO(akushnir): improve error message and include attr name and entire expression in Ivy const message = ivyEnabled ? - `Previous value: 'initial'. Current value: 'changed'` : + `Previous value for 'attr.id': 'Expressions: a and initial!'. Current value: 'Expressions: a and changed!'` : `Previous value: 'id: Expressions: a and initial!'. Current value: 'id: Expressions: a and changed!'`; expect( () => initWithTemplate(