diff --git a/goldens/public-api/core/errors.md b/goldens/public-api/core/errors.md index 4aeefda8d7940..f29525b2c1f29 100644 --- a/goldens/public-api/core/errors.md +++ b/goldens/public-api/core/errors.md @@ -57,6 +57,8 @@ export const enum RuntimeErrorCode { // (undocumented) IMPORT_PROVIDERS_FROM_STANDALONE = 800, // (undocumented) + INFINITE_CHANGE_DETECTION = 103, + // (undocumented) INJECTOR_ALREADY_DESTROYED = 205, // (undocumented) INVALID_DIFFER_INPUT = 900, diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 331f765de694d..0adf48ccf1cb8 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -31,6 +31,7 @@ export const enum RuntimeErrorCode { EXPRESSION_CHANGED_AFTER_CHECKED = -100, RECURSIVE_APPLICATION_REF_TICK = 101, RECURSIVE_APPLICATION_RENDER = 102, + INFINITE_CHANGE_DETECTION = 103, // Dependency Injection Errors CYCLIC_DI_DEPENDENCY = -200, diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts index a5f3423c1676b..8fe630464d2de 100644 --- a/packages/core/src/render3/instructions/change_detection.ts +++ b/packages/core/src/render3/instructions/change_detection.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {RuntimeError, RuntimeErrorCode} from '../../errors'; import {assertDefined, assertEqual} from '../../util/assert'; import {assertLContainer} from '../assert'; import {getComponentViewByInstance} from '../context_discovery'; @@ -19,6 +20,11 @@ import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, mar import {executeTemplate, executeViewQueryFn, handleError, processHostBindingOpCodes, refreshContentQueries} from './shared'; +/** + * The maximum number of times the change detection traversal will rerun before throwing an error. + */ +const MAXIMUM_REFRESH_RERUNS = 100; + export function detectChangesInternal( tView: TView, lView: LView, context: T, notifyErrorHandler = true) { const environment = lView[ENVIRONMENT]; @@ -37,6 +43,25 @@ export function detectChangesInternal( try { refreshView(tView, lView, tView.template, context); + let retries = 0; + // If after running change detection, this view still needs to be refreshed or there are + // descendants views that need to be refreshed due to re-dirtying during the change detection + // run, detect changes on the view again. We run change detection in `Targeted` mode to only + // refresh views with the `RefreshView` flag. + while (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh)) { + if (retries === MAXIMUM_REFRESH_RERUNS) { + throw new RuntimeError( + RuntimeErrorCode.INFINITE_CHANGE_DETECTION, + ngDevMode && + 'Infinite change detection while trying to refresh views. ' + + 'There may be components which each cause the other to require a refresh, ' + + 'causing an infinite loop.'); + } + retries++; + // Even if this view is detached, we still detect changes in targeted mode because this was + // the root of the change detection run. + detectChangesInView(lView, ChangeDetectionMode.Targeted); + } } catch (error) { if (notifyErrorHandler) { handleError(lView, error); @@ -228,7 +253,6 @@ export function refreshView( if (!isInCheckNoChangesPass) { lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); } - lView[FLAGS] &= ~LViewFlags.RefreshView; } catch (e) { // If refreshing a view causes an error, we need to remark the ancestors as needing traversal // because the error might have caused a situation where views below the current location are @@ -319,7 +343,7 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) { // Flag cleared before change detection runs so that the view can be re-marked for traversal if // necessary. - lView[FLAGS] &= ~LViewFlags.HasChildViewsToRefresh; + lView[FLAGS] &= ~(LViewFlags.HasChildViewsToRefresh | LViewFlags.RefreshView); if ((flags & (LViewFlags.CheckAlways | LViewFlags.Dirty) && mode === ChangeDetectionMode.Global) || diff --git a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts index 3eba4179097f2..600b92b2f1871 100644 --- a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts +++ b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts @@ -410,35 +410,25 @@ describe('change detection for transplanted views', () => { expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); }); - it('updates only the declaration view when there is a change to declaration and declaration is marked dirty', - () => { - appComponent.declaration.name = 'new name'; - appComponent.declaration.changeDetectorRef.markForCheck(); - fixture.detectChanges(false); - - const expectedContent = - 'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(new name)'; - expect(fixture.nativeElement.textContent).toEqual(expectedContent); - expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0); + it('updates the declaration view when there is a change to either declaration or insertion', () => { + appComponent.declaration.name = 'new name'; + appComponent.declaration.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); - // Note here that this second change detection should not be necessary, but is because of - // the backwards reference not being fully supported. The assertions below should be true - // after the first CD. - fixture.detectChanges(false); - expect(fixture.nativeElement.textContent) - .toEqual( - 'Insertion(initial)TemplateDeclaration(new name)TemplateContext(initial)Declaration(new name)'); - expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); - }); + const expectedContent = + 'Insertion(initial)TemplateDeclaration(new name)TemplateContext(initial)Declaration(new name)'; + expect(fixture.nativeElement.textContent).toEqual(expectedContent); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); + }); - it('should not update when there is a change to insertion and declaration is marked dirty', () => { + it('should update when there is a change to insertion and declaration is marked dirty', () => { appComponent.insertion.name = 'new name'; appComponent.declaration.changeDetectorRef.markForCheck(); fixture.detectChanges(false); expect(fixture.nativeElement.textContent) .toEqual( 'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)'); - expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); }); it('should update insertion view and template when there is a change to insertion and insertion marked dirty', @@ -462,8 +452,9 @@ describe('change detection for transplanted views', () => { appComponent.insertion.changeDetectorRef.markForCheck(); fixture.detectChanges(false); expect(appComponent.declaration.transplantedViewRefreshCount) - .withContext('Should refresh once because backwards references are not rechecked') - .toEqual(1); + .withContext( + 'Should refresh twice because insertion executes and then declaration marks transplanted view dirty again') + .toEqual(2); }); }); @@ -653,7 +644,8 @@ describe('change detection for transplanted views', () => { // Detach view, manually call `detectChanges`, and verify the template was refreshed component.rootViewContainerRef.detach(); viewRef.detectChanges(); - expect(component.templateExecutions).toEqual(1); + // This view is a backwards reference so it's refreshed twice + expect(component.templateExecutions).toEqual(2); }); it('should work when change detecting detached transplanted view already marked for refresh', @@ -669,7 +661,8 @@ describe('change detection for transplanted views', () => { // should not affect parent counters. viewRef.detectChanges(); }).not.toThrow(); - expect(component.templateExecutions).toEqual(1); + // This view is a backwards reference so it's refreshed twice + expect(component.templateExecutions).toEqual(2); }); it('should work when re-inserting a previously detached transplanted view marked for refresh', @@ -689,7 +682,10 @@ describe('change detection for transplanted views', () => { // the counter, this would fail when attempted to decrement. fixture.detectChanges(false); }).not.toThrow(); - expect(component.templateExecutions).toBeGreaterThan(0); + // The transplanted view gets refreshed twice because it's actually inserted "backwards" + // The view is defined in AppComponent but inserted in its ViewContainerRef (as an + // embedded view in AppComponent's host view). + expect(component.templateExecutions).toEqual(2); }); it('should work when detaching an attached transplanted view with the refresh flag', () => { diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index 4ca4f24c3d08d..bf18698032168 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -833,6 +833,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index b388ab135e0f7..ed8bae70b2c41 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -899,6 +899,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 339526394f376..1b94eab2686fc 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -668,6 +668,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index 66c87fefe52b1..a70307e78288d 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -248,6 +248,9 @@ { "name": "MATH_ML_NAMESPACE" }, + { + "name": "MAXIMUM_REFRESH_RERUNS" + }, { "name": "MINIMUM_SLOT" }, @@ -752,6 +755,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 2541c49cac9ad..c96bccee489ce 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -911,6 +911,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, 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 c6f8a4f81f548..94a46354913fc 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 @@ -881,6 +881,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index abb62e44cc22b..12b3954c8c110 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -527,6 +527,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 4468b84fc6218..95775b384467c 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -725,6 +725,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 80b9dd7a73b92..fca3e5849afa6 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1166,6 +1166,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 9f8c2ecd96114..3bace25ebec6a 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -596,6 +596,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index b1adad7c6d147..9cc524c38398c 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -797,6 +797,9 @@ { "name": "detectChangesInEmbeddedViews" }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInViewIfAttached" },