X Tutup
Skip to content

feat(ivy): improve ExpressionChangedAfterChecked error message for attributes #34505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AndrewKushnir wants to merge 1 commit into from
Closed

feat(ivy): improve ExpressionChangedAfterChecked error message for attributes #34505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/core/src/render3/instructions/attribute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Copy Markdown
Member

@pkozlowski-opensource pkozlowski-opensource Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: what do you think of renaming storePropertyBindingMetadata to storeBindingMetadata (since now it is applied to both property bindings, attributes, interpolations etc.)?




Expand All @@ -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;
}
74 changes: 54 additions & 20 deletions packages/core/src/render3/instructions/attribute_interpolation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';



Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
8 changes: 4 additions & 4 deletions packages/core/src/render3/instructions/property_interpolation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 4 additions & 5 deletions packages/core/test/acceptance/change_detection_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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('<div [attr.id]="unstableStringExpression"></div>'))
.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(
Expand Down
X Tutup