Skip to content

Use consumer dirtiness to drive whether a view should refresh #52476

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
atscott wants to merge 3 commits into from
Closed

Use consumer dirtiness to drive whether a view should refresh #52476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
133b9ee
refactor(core): Remove warning about signal set during template execu…
atscott Oct 26, 2023
808abb1
refactor(core): Update LView consumer to use only 1 consumer for a co…
atscott Oct 24, 2023
7bd221f
refactor(core): Do not refresh view if producers did not actually change
atscott Oct 26, 2023
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
3 changes: 3 additions & 0 deletions goldens/public-api/core/primitives/signals/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ export function consumerBeforeComputation(node: ReactiveNode | null): ReactiveNo
// @public
export function consumerDestroy(node: ReactiveNode): void;

// @public
export function consumerPollProducersForChange(node: ReactiveNode): boolean;

// @public
export function createComputed<T>(computation: () => T): ComputedGetter<T>;

Expand Down
2 changes: 1 addition & 1 deletion packages/core/primitives/signals/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
export {createComputed} from './src/computed';
export {defaultEquals, ValueEqualityFn} from './src/equality';
export {setThrowInvalidWriteToSignalError} from './src/errors';
export {consumerAfterComputation, consumerBeforeComputation, consumerDestroy, getActiveConsumer, isInNotificationPhase, isReactive, producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, producerUpdateValueVersion, Reactive, REACTIVE_NODE, ReactiveNode, setActiveConsumer, SIGNAL} from './src/graph';
export {consumerAfterComputation, consumerBeforeComputation, consumerDestroy, consumerPollProducersForChange, getActiveConsumer, isInNotificationPhase, isReactive, producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, producerUpdateValueVersion, Reactive, REACTIVE_NODE, ReactiveNode, setActiveConsumer, SIGNAL} from './src/graph';
export {createSignal, setPostSignalSetFn, SignalGetter, signalMutateFn, SignalNode, signalSetFn, signalUpdateFn} from './src/signal';
export {createWatch, Watch, WatchCleanupFn, WatchCleanupRegisterFn} from './src/watch';
export {setAlternateWeakRefImpl} from './src/weak_ref';
56 changes: 34 additions & 22 deletions packages/core/src/defer/instructions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {setActiveConsumer} from '@angular/core/primitives/signals';

import {InjectionToken, Injector} from '../di';
import {RuntimeError, RuntimeErrorCode} from '../errors';
import {findMatchingDehydratedView} from '../hydration/views';
Expand Down Expand Up @@ -179,21 +181,26 @@ export function ɵɵdeferWhen(rawValue: unknown) {
const lView = getLView();
const bindingIndex = nextBindingIndex();
if (bindingUpdated(lView, bindingIndex, rawValue)) {
const value = Boolean(rawValue); // handle truthy or falsy values
const tNode = getSelectedTNode();
const lDetails = getLDeferBlockDetails(lView, tNode);
const renderedState = lDetails[DEFER_BLOCK_STATE];
if (value === false && renderedState === DeferBlockInternalState.Initial) {
// If nothing is rendered yet, render a placeholder (if defined).
renderPlaceholder(lView, tNode);
} else if (
value === true &&
(renderedState === DeferBlockInternalState.Initial ||
renderedState === DeferBlockState.Placeholder)) {
// The `when` condition has changed to `true`, trigger defer block loading
// if the block is either in initial (nothing is rendered) or a placeholder
// state.
triggerDeferBlock(lView, tNode);
const prevConsumer = setActiveConsumer(null);
try {
const value = Boolean(rawValue); // handle truthy or falsy values
const tNode = getSelectedTNode();
const lDetails = getLDeferBlockDetails(lView, tNode);
const renderedState = lDetails[DEFER_BLOCK_STATE];
if (value === false && renderedState === DeferBlockInternalState.Initial) {
// If nothing is rendered yet, render a placeholder (if defined).
renderPlaceholder(lView, tNode);
} else if (
value === true &&
(renderedState === DeferBlockInternalState.Initial ||
renderedState === DeferBlockState.Placeholder)) {
// The `when` condition has changed to `true`, trigger defer block loading
// if the block is either in initial (nothing is rendered) or a placeholder
// state.
triggerDeferBlock(lView, tNode);
}
} finally {
setActiveConsumer(prevConsumer);
}
}
}
Expand All @@ -207,13 +214,18 @@ export function ɵɵdeferPrefetchWhen(rawValue: unknown) {
const bindingIndex = nextBindingIndex();

if (bindingUpdated(lView, bindingIndex, rawValue)) {
const value = Boolean(rawValue); // handle truthy or falsy values
const tView = lView[TVIEW];
const tNode = getSelectedTNode();
const tDetails = getTDeferBlockDetails(tView, tNode);
if (value === true && tDetails.loadingState === DeferDependenciesLoadingState.NOT_STARTED) {
// If loading has not been started yet, trigger it now.
triggerPrefetching(tDetails, lView, tNode);
const prevConsumer = setActiveConsumer(null);
try {
const value = Boolean(rawValue); // handle truthy or falsy values
const tView = lView[TVIEW];
const tNode = getSelectedTNode();
const tDetails = getTDeferBlockDetails(tView, tNode);
if (value === true && tDetails.loadingState === DeferDependenciesLoadingState.NOT_STARTED) {
// If loading has not been started yet, trigger it now.
triggerPrefetching(tDetails, lView, tNode);
}
} finally {
setActiveConsumer(prevConsumer);
}
}
}
Expand Down
105 changes: 76 additions & 29 deletions packages/core/src/render3/instructions/change_detection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {consumerAfterComputation, consumerBeforeComputation, consumerPollProducersForChange, ReactiveNode} from '@angular/core/primitives/signals';

import {RuntimeError, RuntimeErrorCode} from '../../errors';
import {assertDefined, assertEqual} from '../../util/assert';
import {assertLContainer} from '../assert';
Expand All @@ -14,6 +16,7 @@ import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} fr
import {CONTAINER_HEADER_OFFSET, HAS_CHILD_VIEWS_TO_REFRESH, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container';
import {ComponentTemplate, RenderFlags} from '../interfaces/definition';
import {CONTEXT, EFFECTS_TO_SCHEDULE, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView, TViewType} from '../interfaces/view';
import {getOrBorrowReactiveLViewConsumer, maybeReturnReactiveLViewConsumer, ReactiveLViewConsumer} from '../reactive_lview_consumer';
import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state';
import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils';
import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
Expand Down Expand Up @@ -49,7 +52,8 @@ export function detectChangesInternal<T>(
// 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)) {
while (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) ||
lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty) {
if (retries === MAXIMUM_REFRESH_RERUNS) {
throw new RuntimeError(
RuntimeErrorCode.INFINITE_CHANGE_DETECTION,
Expand Down Expand Up @@ -145,7 +149,18 @@ export function refreshView<T>(

!isInCheckNoChangesPass && lView[ENVIRONMENT].inlineEffectRunner?.flush();


// Start component reactive context
// - We might already be in a reactive context if this is an embedded view of the host.
// - We might be descending into a view that needs a consumer.
enterView(lView);
let prevConsumer: ReactiveNode|null = null;
let currentConsumer: ReactiveLViewConsumer|null = null;
if (!isInCheckNoChangesPass && viewShouldHaveReactiveConsumer(tView)) {
currentConsumer = getOrBorrowReactiveLViewConsumer(lView);
prevConsumer = consumerBeforeComputation(currentConsumer);
}

try {
resetPreOrderHookFlags(lView);

Expand All @@ -160,23 +175,17 @@ export function refreshView<T>(
// execute pre-order hooks (OnInit, OnChanges, DoCheck)
// PERF WARNING: do NOT extract this to a separate function without running benchmarks
if (!isInCheckNoChangesPass) {
const consumer = lView[REACTIVE_TEMPLATE_CONSUMER];
try {
consumer && (consumer.isRunning = true);
if (hooksInitPhaseCompleted) {
const preOrderCheckHooks = tView.preOrderCheckHooks;
if (preOrderCheckHooks !== null) {
executeCheckHooks(lView, preOrderCheckHooks, null);
}
} else {
const preOrderHooks = tView.preOrderHooks;
if (preOrderHooks !== null) {
executeInitAndCheckHooks(lView, preOrderHooks, InitPhaseState.OnInitHooksToBeRun, null);
}
incrementInitPhaseFlags(lView, InitPhaseState.OnInitHooksToBeRun);
if (hooksInitPhaseCompleted) {
const preOrderCheckHooks = tView.preOrderCheckHooks;
if (preOrderCheckHooks !== null) {
executeCheckHooks(lView, preOrderCheckHooks, null);
}
} else {
const preOrderHooks = tView.preOrderHooks;
if (preOrderHooks !== null) {
executeInitAndCheckHooks(lView, preOrderHooks, InitPhaseState.OnInitHooksToBeRun, null);
}
} finally {
consumer && (consumer.isRunning = false);
incrementInitPhaseFlags(lView, InitPhaseState.OnInitHooksToBeRun);
}
}

Expand Down Expand Up @@ -279,10 +288,32 @@ export function refreshView<T>(
markAncestorsForTraversal(lView);
throw e;
} finally {
if (currentConsumer !== null) {
consumerAfterComputation(currentConsumer, prevConsumer);
maybeReturnReactiveLViewConsumer(currentConsumer);
}
leaveView();
}
}

/**
* Indicates if the view should get its own reactive consumer node.
*
* In the current design, all embedded views share a consumer with the component view. This allows
* us to refresh at the component level rather than at a per-view level. In addition, root views get
* their own reactive node because root component will have a host view that executes the
* component's host bindings. This needs to be tracked in a consumer as well.
*
* To get a more granular change detection than per-component, all we would just need to update the
* condition here so that a given view gets a reactive consumer which can become dirty independently
* from its parent component. For example embedded views for signal components could be created with
* a new type "SignalEmbeddedView" and the condition here wouldn't even need updating in order to
* get granular per-view change detection for signal components.
*/
function viewShouldHaveReactiveConsumer(tView: TView) {
return tView.type !== TViewType.Embedded;
}

/**
* Goes over embedded views (ones created through ViewContainerRef APIs) and refreshes
* them by executing an associated template function.
Expand Down Expand Up @@ -358,20 +389,36 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode();
const tView = lView[TVIEW];
const flags = lView[FLAGS];

// Flag cleared before change detection runs so that the view can be re-marked for traversal if
// necessary.
const consumer = lView[REACTIVE_TEMPLATE_CONSUMER];

// Refresh CheckAlways views in Global mode.
let shouldRefreshView: boolean =
!!(mode === ChangeDetectionMode.Global && flags & LViewFlags.CheckAlways);

// Refresh Dirty views in Global mode, as long as we're not in checkNoChanges.
// CheckNoChanges never worked with `OnPush` components because the `Dirty` flag was
// cleared before checkNoChanges ran. Because there is now a loop for to check for
// backwards views, it gives an opportunity for `OnPush` components to be marked `Dirty`
// before the CheckNoChanges pass. We don't want existing errors that are hidden by the
// current CheckNoChanges bug to surface when making unrelated changes.
shouldRefreshView ||=
!!(flags & LViewFlags.Dirty && mode === ChangeDetectionMode.Global &&
(!isInCheckNoChangesPass || RUN_IN_CHECK_NO_CHANGES_ANYWAY));

// Always refresh views marked for refresh, regardless of mode.
shouldRefreshView ||= !!(flags & LViewFlags.RefreshView);

// Refresh views when they have a dirty reactive consumer, regardless of mode.
shouldRefreshView ||= !!(consumer?.dirty && consumerPollProducersForChange(consumer));

// Mark the Flags and `ReactiveNode` as not dirty before refreshing the component, so that they
// can be re-dirtied during the refresh process.
if (consumer) {
consumer.dirty = false;
}
lView[FLAGS] &= ~(LViewFlags.HasChildViewsToRefresh | LViewFlags.RefreshView);

if ((flags & LViewFlags.CheckAlways && mode === ChangeDetectionMode.Global) ||
(flags & LViewFlags.Dirty && mode === ChangeDetectionMode.Global &&
// CheckNoChanges never worked with `OnPush` components because the `Dirty` flag was cleared
// before checkNoChanges ran. Because there is now a loop for to check for backwards views,
// it gives an opportunity for `OnPush` components to be marked `Dirty` before the
// CheckNoChanges pass. We don't want existing errors that are hidden by the current
// CheckNoChanges bug to surface when making unrelated changes.
(!isInCheckNoChangesPass || RUN_IN_CHECK_NO_CHANGES_ANYWAY)) ||
flags & LViewFlags.RefreshView) {
if (shouldRefreshView) {
refreshView(tView, lView, tView.template, lView[CONTEXT]);
} else if (flags & LViewFlags.HasChildViewsToRefresh) {
detectChangesInEmbeddedViews(lView, ChangeDetectionMode.Targeted);
Expand Down
118 changes: 65 additions & 53 deletions packages/core/src/render3/instructions/control_flow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {setActiveConsumer} from '@angular/core/primitives/signals';

import {TrackByFunction} from '../../change_detection';
import {DehydratedContainerView} from '../../hydration/interfaces';
import {findMatchingDehydratedView} from '../../hydration/views';
Expand Down Expand Up @@ -48,22 +50,27 @@ export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex
const viewInContainerIdx = 0;

if (bindingUpdated(hostLView, bindingIndex, matchingTemplateIndex)) {
// The index of the view to show changed - remove the previously displayed one
// (it is a noop if there are no active views in a container).
removeLViewFromLContainer(lContainer, viewInContainerIdx);

// Index -1 is a special case where none of the conditions evaluates to
// a truthy value and as the consequence we've got no view to show.
if (matchingTemplateIndex !== -1) {
const templateTNode = getExistingTNode(hostLView[TVIEW], matchingTemplateIndex);

const dehydratedView = findMatchingDehydratedView(lContainer, templateTNode.tView!.ssrId);
const embeddedLView =
createAndRenderEmbeddedLView(hostLView, templateTNode, value, {dehydratedView});

addLViewToLContainer(
lContainer, embeddedLView, viewInContainerIdx,
shouldAddViewToDom(templateTNode, dehydratedView));
const prevConsumer = setActiveConsumer(null);
try {
// The index of the view to show changed - remove the previously displayed one
// (it is a noop if there are no active views in a container).
removeLViewFromLContainer(lContainer, viewInContainerIdx);

// Index -1 is a special case where none of the conditions evaluates to
// a truthy value and as the consequence we've got no view to show.
if (matchingTemplateIndex !== -1) {
const templateTNode = getExistingTNode(hostLView[TVIEW], matchingTemplateIndex);

const dehydratedView = findMatchingDehydratedView(lContainer, templateTNode.tView!.ssrId);
const embeddedLView =
createAndRenderEmbeddedLView(hostLView, templateTNode, value, {dehydratedView});

addLViewToLContainer(
lContainer, embeddedLView, viewInContainerIdx,
shouldAddViewToDom(templateTNode, dehydratedView));
}
} finally {
setActiveConsumer(prevConsumer);
}
} else {
// We might keep displaying the same template but the actual value of the expression could have
Expand Down Expand Up @@ -238,46 +245,51 @@ class LiveCollectionLContainerImpl extends
*/
export function ɵɵrepeater(
metadataSlotIdx: number, collection: Iterable<unknown>|undefined|null): void {
const hostLView = getLView();
const hostTView = hostLView[TVIEW];
const metadata = hostLView[HEADER_OFFSET + metadataSlotIdx] as RepeaterMetadata;

if (metadata.liveCollection === undefined) {
const containerIndex = metadataSlotIdx + 1;
const lContainer = getLContainer(hostLView, HEADER_OFFSET + containerIndex);
const itemTemplateTNode = getExistingTNode(hostTView, containerIndex);
metadata.liveCollection =
new LiveCollectionLContainerImpl(lContainer, hostLView, itemTemplateTNode);
} else {
metadata.liveCollection.reset();
}
const prevConsumer = setActiveConsumer(null);
try {
const hostLView = getLView();
const hostTView = hostLView[TVIEW];
const metadata = hostLView[HEADER_OFFSET + metadataSlotIdx] as RepeaterMetadata;

if (metadata.liveCollection === undefined) {
const containerIndex = metadataSlotIdx + 1;
const lContainer = getLContainer(hostLView, HEADER_OFFSET + containerIndex);
const itemTemplateTNode = getExistingTNode(hostTView, containerIndex);
metadata.liveCollection =
new LiveCollectionLContainerImpl(lContainer, hostLView, itemTemplateTNode);
} else {
metadata.liveCollection.reset();
}

const liveCollection = metadata.liveCollection;
reconcile(liveCollection, collection, metadata.trackByFn);

// moves in the container might caused context's index to get out of order, re-adjust if needed
liveCollection.updateIndexes();

// handle empty blocks
if (metadata.hasEmptyBlock) {
const bindingIndex = nextBindingIndex();
const isCollectionEmpty = liveCollection.length === 0;
if (bindingUpdated(hostLView, bindingIndex, isCollectionEmpty)) {
const emptyTemplateIndex = metadataSlotIdx + 2;
const lContainerForEmpty = getLContainer(hostLView, HEADER_OFFSET + emptyTemplateIndex);
if (isCollectionEmpty) {
const emptyTemplateTNode = getExistingTNode(hostTView, emptyTemplateIndex);
const dehydratedView =
findMatchingDehydratedView(lContainerForEmpty, emptyTemplateTNode.tView!.ssrId);
const embeddedLView = createAndRenderEmbeddedLView(
hostLView, emptyTemplateTNode, undefined, {dehydratedView});
addLViewToLContainer(
lContainerForEmpty, embeddedLView, 0,
shouldAddViewToDom(emptyTemplateTNode, dehydratedView));
} else {
removeLViewFromLContainer(lContainerForEmpty, 0);
const liveCollection = metadata.liveCollection;
reconcile(liveCollection, collection, metadata.trackByFn);

// moves in the container might caused context's index to get out of order, re-adjust if needed
liveCollection.updateIndexes();

// handle empty blocks
if (metadata.hasEmptyBlock) {
const bindingIndex = nextBindingIndex();
const isCollectionEmpty = liveCollection.length === 0;
if (bindingUpdated(hostLView, bindingIndex, isCollectionEmpty)) {
const emptyTemplateIndex = metadataSlotIdx + 2;
const lContainerForEmpty = getLContainer(hostLView, HEADER_OFFSET + emptyTemplateIndex);
if (isCollectionEmpty) {
const emptyTemplateTNode = getExistingTNode(hostTView, emptyTemplateIndex);
const dehydratedView =
findMatchingDehydratedView(lContainerForEmpty, emptyTemplateTNode.tView!.ssrId);
const embeddedLView = createAndRenderEmbeddedLView(
hostLView, emptyTemplateTNode, undefined, {dehydratedView});
addLViewToLContainer(
lContainerForEmpty, embeddedLView, 0,
shouldAddViewToDom(emptyTemplateTNode, dehydratedView));
} else {
removeLViewFromLContainer(lContainerForEmpty, 0);
}
}
}
} finally {
setActiveConsumer(prevConsumer);
}
}

Expand Down
Loading