fix(core): don’t throw if queries change during change detection.

Throwing on query changes would be a breaking change compared to v2.
Also discovers a bug with querying manually projected content, see #15117.

Related to #14748
Closes #14925
This commit is contained in:
Tobias Bosch 2017-03-13 11:12:13 -07:00 committed by Chuck Jazdzewski
parent 959a03a61f
commit 06fc42bc44
3 changed files with 25 additions and 35 deletions

View File

@ -318,12 +318,10 @@ function createViewNodes(view: ViewData) {
export function checkNoChangesView(view: ViewData) {
Services.updateDirectives(view, CheckType.CheckNoChanges);
execEmbeddedViewsAction(view, ViewAction.CheckNoChanges);
execQueriesAction(
view, NodeFlags.TypeContentQuery, NodeFlags.DynamicQuery, CheckType.CheckNoChanges);
Services.updateRenderer(view, CheckType.CheckNoChanges);
execComponentViewsAction(view, ViewAction.CheckNoChanges);
execQueriesAction(
view, NodeFlags.TypeViewQuery, NodeFlags.DynamicQuery, CheckType.CheckNoChanges);
// Note: We don't check queries for changes as we didn't do this in v2.x.
// TODO(tbosch): investigate if we can enable the check again in v5.x with a nicer error message.
}
export function checkAndUpdateView(view: ViewData) {

View File

@ -536,6 +536,29 @@ export function main() {
expect(q.query.length).toBe(0);
});
it('should not throw if a content template is queried and created in the view during change detection',
() => {
@Component(
{selector: 'auto-projecting', template: '<div *ngIf="true; then: content"></div>'})
class AutoProjecting {
@ContentChild(TemplateRef)
content: TemplateRef<any>;
@ContentChildren(TextDirective)
query: QueryList<TextDirective>;
}
TestBed.configureTestingModule({declarations: [AutoProjecting]});
const template =
'<auto-projecting #q><ng-template><div text="1"></div></ng-template></auto-projecting>';
const view = createTestCmpAndDetectChanges(MyComp0, template);
const q = view.debugElement.children[0].references['q'];
// This should be 1, but due to
// https://github.com/angular/angular/issues/15117 this is 0.
expect(q.query.length).toBe(0);
});
});
});
}

View File

@ -369,37 +369,6 @@ export function main() {
});
describe('general binding behavior', () => {
it('should checkNoChanges', () => {
const {view} = createAndGetRootNodes(compViewDef([
elementDef(NodeFlags.None, null, null, 3, 'div'),
...contentQueryProviders(),
anchorDef(NodeFlags.EmbeddedViews, null, null, 0, null, embeddedViewDef([
elementDef(NodeFlags.None, null, null, 1, 'div'),
aServiceProvider(),
])),
]));
Services.checkAndUpdateView(view);
Services.checkNoChangesView(view);
const childView = Services.createEmbeddedView(view, view.def.nodes[3]);
attachEmbeddedView(view, asElementData(view, 3), 0, childView);
let err: any;
try {
Services.checkNoChangesView(view);
} catch (e) {
err = e;
}
expect(err).toBeTruthy();
expect(err.message)
.toBe(
`ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'Query 1 not dirty'. Current value: 'Query 1 dirty'.`);
const debugCtx = getDebugContext(err);
expect(debugCtx.view).toBe(view);
expect(debugCtx.nodeIndex).toBe(2);
});
it('should report debug info on binding errors', () => {
class QueryService {
set a(value: any) { throw new Error('Test'); }