fix(core): make sure that lifecycle hooks are not tracked (#49701)

Angular lifecycle hooks should never be run as part of the reactive
context: we do not expect that signal reads in lifecycle hooks
report to any consumers.

In the current Angular some of the lifecycle hooks can be flushed
early, while executting template update pass. We need to make sure
that signal reads in those lifecycle hooks do not register as part
of the effect that marks components for check.
"

PR Close #49701
This commit is contained in:
Pawel Kozlowski 2023-04-04 15:49:38 +02:00 committed by Dylan Hunn
parent a4e749ffca
commit df1dfc4c17
10 changed files with 76 additions and 13 deletions

View file

@ -7,6 +7,7 @@
*/
import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, DoCheck, OnChanges, OnDestroy, OnInit} from '../interface/lifecycle_hooks';
import {setActiveConsumer} from '../signals';
import {assertDefined, assertEqual, assertNotEqual} from '../util/assert';
import {assertFirstCreatePass} from './assert';
@ -238,6 +239,22 @@ function callHooks(
}
}
/**
* Executes a single lifecycle hook, making sure that:
* - it is called in the non-reactive context;
* - profiling data are registered.
*/
function callHookInternal(directive: any, hook: () => void) {
profiler(ProfilerEvent.LifecycleHookStart, directive, hook);
const prevConsumer = setActiveConsumer(null);
try {
hook.call(directive);
} finally {
setActiveConsumer(prevConsumer);
profiler(ProfilerEvent.LifecycleHookEnd, directive, hook);
}
}
/**
* Execute one hook against the current `LView`.
*
@ -258,19 +275,9 @@ function callHook(currentView: LView, initPhase: InitPhaseState, arr: HookData,
(currentView[PREORDER_HOOK_FLAGS] >> PreOrderHookFlags.NumberOfInitHooksCalledShift) &&
(currentView[FLAGS] & LViewFlags.InitPhaseStateMask) === initPhase) {
currentView[FLAGS] += LViewFlags.IndexWithinInitPhaseIncrementer;
profiler(ProfilerEvent.LifecycleHookStart, directive, hook);
try {
hook.call(directive);
} finally {
profiler(ProfilerEvent.LifecycleHookEnd, directive, hook);
}
callHookInternal(directive, hook);
}
} else {
profiler(ProfilerEvent.LifecycleHookStart, directive, hook);
try {
hook.call(directive);
} finally {
profiler(ProfilerEvent.LifecycleHookEnd, directive, hook);
}
callHookInternal(directive, hook);
}
}

View file

@ -767,6 +767,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -572,6 +572,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -770,6 +770,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -752,6 +752,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -440,6 +440,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -950,6 +950,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -521,6 +521,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -680,6 +680,9 @@
{
"name": "callHook"
},
{
"name": "callHookInternal"
},
{
"name": "callHooks"
},

View file

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AfterViewInit, Component, destroyPlatform, effect, inject, Injector, NgZone, signal} from '@angular/core';
import {AfterViewInit, Component, destroyPlatform, effect, inject, Injector, Input, NgZone, OnChanges, signal, SimpleChanges} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {bootstrapApplication} from '@angular/platform-browser';
import {withBody} from '@angular/private/testing';
@ -216,4 +216,36 @@ describe('effects', () => {
await bootstrapApplication(Cmp);
}));
it('should allow writing to signals in ngOnChanges', () => {
@Component({
selector: 'with-input',
standalone: true,
template: '{{inSignal()}}',
})
class WithInput implements OnChanges {
inSignal = signal<string|undefined>(undefined);
@Input() in : string|undefined;
ngOnChanges(changes: SimpleChanges): void {
if (changes.in) {
this.inSignal.set(changes.in.currentValue);
}
}
}
@Component({
selector: 'test-cmp',
standalone: true,
imports: [WithInput],
template: `<with-input [in]="'A'" />|<with-input [in]="'B'" />
`,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('A|B');
});
});