Skip to content

Commit 5685b4c

Browse files
arturovtthePunderWoman
authored andcommitted
refactor(core): prevent duplicating LView destroyed checks (angular#59387)
The `type_checks` module already exposes a utility function that checks whether `LView` is marked as destroyed. There is no need to check flags in other places, as we can reuse the helper function. PR Close angular#59387
1 parent aaf1fbb commit 5685b4c

File tree

16 files changed

+32
-19
lines changed

16 files changed

+32
-19
lines changed

packages/core/src/render3/instructions/change_detection.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import {
6969
refreshContentQueries,
7070
} from './shared';
7171
import {runEffectsInView} from '../reactivity/view_effect_runner';
72+
import {isDestroyed} from '../interfaces/type_checks';
7273

7374
/**
7475
* The maximum number of times the change detection traversal will rerun before throwing an error.
@@ -193,8 +194,10 @@ export function refreshView<T>(
193194
context: T,
194195
) {
195196
ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode');
197+
198+
if (isDestroyed(lView)) return;
199+
196200
const flags = lView[FLAGS];
197-
if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
198201

199202
// Check no changes mode is a dev only mode used to verify that bindings have not changed
200203
// since they were assigned. We do not want to execute lifecycle hooks in that mode.

packages/core/src/render3/interfaces/type_checks.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,6 @@ export function hasI18n(lView: LView): boolean {
5757
}
5858

5959
export function isDestroyed(lView: LView): boolean {
60+
// Determines whether a given LView is marked as destroyed.
6061
return (lView[FLAGS] & LViewFlags.Destroyed) === LViewFlags.Destroyed;
6162
}

packages/core/src/render3/node_manipulation.ts

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {
10-
consumerDestroy,
11-
getActiveConsumer,
12-
setActiveConsumer,
13-
} from '@angular/core/primitives/signals';
9+
import {consumerDestroy, setActiveConsumer} from '@angular/core/primitives/signals';
1410

1511
import {NotificationSource} from '../change_detection/scheduling/zoneless_scheduling';
1612
import {hasInSkipHydrationBlockFlag} from '../hydration/skip_hydration';
@@ -55,8 +51,8 @@ import {
5551
TProjectionNode,
5652
} from './interfaces/node';
5753
import {Renderer} from './interfaces/renderer';
58-
import {RComment, RElement, RNode, RTemplate, RText} from './interfaces/renderer_dom';
59-
import {isLContainer, isLView} from './interfaces/type_checks';
54+
import {RComment, RElement, RNode, RText} from './interfaces/renderer_dom';
55+
import {isDestroyed, isLContainer, isLView} from './interfaces/type_checks';
6056
import {
6157
CHILD_HEAD,
6258
CLEANUP,
@@ -445,15 +441,17 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView |
445441
* @param lView The view to be destroyed.
446442
*/
447443
export function destroyLView(tView: TView, lView: LView) {
448-
if (!(lView[FLAGS] & LViewFlags.Destroyed)) {
449-
const renderer = lView[RENDERER];
444+
if (isDestroyed(lView)) {
445+
return;
446+
}
450447

451-
if (renderer.destroyNode) {
452-
applyView(tView, lView, renderer, WalkTNodeTreeAction.Destroy, null, null);
453-
}
448+
const renderer = lView[RENDERER];
454449

455-
destroyViewTree(lView);
450+
if (renderer.destroyNode) {
451+
applyView(tView, lView, renderer, WalkTNodeTreeAction.Destroy, null, null);
456452
}
453+
454+
destroyViewTree(lView);
457455
}
458456

459457
/**
@@ -465,7 +463,7 @@ export function destroyLView(tView: TView, lView: LView) {
465463
* @param lView The LView to clean up
466464
*/
467465
function cleanUpView(tView: TView, lView: LView): void {
468-
if (lView[FLAGS] & LViewFlags.Destroyed) {
466+
if (isDestroyed(lView)) {
469467
return;
470468
}
471469

packages/core/src/render3/util/view_utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {assertLView, assertTNode, assertTNodeForLView} from '../assert';
1919
import {LContainer, TYPE} from '../interfaces/container';
2020
import {TConstants, TNode} from '../interfaces/node';
2121
import {RNode} from '../interfaces/renderer_dom';
22-
import {isLContainer, isLView} from '../interfaces/type_checks';
22+
import {isDestroyed, isLContainer, isLView} from '../interfaces/type_checks';
2323
import {
2424
DECLARATION_VIEW,
2525
ENVIRONMENT,
@@ -271,7 +271,7 @@ export function markAncestorsForTraversal(lView: LView) {
271271
* Stores a LView-specific destroy callback.
272272
*/
273273
export function storeLViewOnDestroy(lView: LView, onDestroyCallback: () => void) {
274-
if ((lView[FLAGS] & LViewFlags.Destroyed) === LViewFlags.Destroyed) {
274+
if (isDestroyed(lView)) {
275275
throw new RuntimeError(
276276
RuntimeErrorCode.VIEW_ALREADY_DESTROYED,
277277
ngDevMode && 'View has already been destroyed.',

packages/core/src/render3/view_ref.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {collectNativeNodes} from './collect_native_nodes';
1818
import {checkNoChangesInternal, detectChangesInternal} from './instructions/change_detection';
1919
import {markViewDirty} from './instructions/mark_view_dirty';
2020
import {CONTAINER_HEADER_OFFSET, VIEW_REFS} from './interfaces/container';
21-
import {isLContainer, isRootView} from './interfaces/type_checks';
21+
import {isDestroyed, isLContainer, isRootView} from './interfaces/type_checks';
2222
import {
2323
CONTEXT,
2424
DECLARATION_LCONTAINER,
@@ -116,7 +116,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
116116
}
117117

118118
get destroyed(): boolean {
119-
return (this._lView[FLAGS] & LViewFlags.Destroyed) === LViewFlags.Destroyed;
119+
return isDestroyed(this._lView);
120120
}
121121

122122
destroy(): void {

packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@
370370
"isContentQueryHost",
371371
"isCssClassMatching",
372372
"isCurrentTNodeParent",
373+
"isDestroyed",
373374
"isElementNode",
374375
"isEnvironmentProviders",
375376
"isFunction",

packages/core/test/bundling/animations/bundle.golden_symbols.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@
394394
"isContentQueryHost",
395395
"isCssClassMatching",
396396
"isCurrentTNodeParent",
397+
"isDestroyed",
397398
"isElementNode",
398399
"isEnvironmentProviders",
399400
"isFunction",

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@
316316
"isContentQueryHost",
317317
"isCssClassMatching",
318318
"isCurrentTNodeParent",
319+
"isDestroyed",
319320
"isEnvironmentProviders",
320321
"isFunction",
321322
"isInlineTemplate",

packages/core/test/bundling/defer/bundle.golden_symbols.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,7 @@
763763
"isContentQueryHost",
764764
"isCssClassMatching",
765765
"isCurrentTNodeParent",
766+
"isDestroyed",
766767
"isDirectiveHost",
767768
"isEnvironmentProviders",
768769
"isFunction",

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@
459459
"isContentQueryHost",
460460
"isCssClassMatching",
461461
"isCurrentTNodeParent",
462+
"isDestroyed",
462463
"isDirectiveHost",
463464
"isEmptyInputValue",
464465
"isEnvironmentProviders",

0 commit comments

Comments
 (0)