From 28905ab9ae51a4f6840d4e557a474a7c0505ede1 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Wed, 24 Apr 2024 16:11:28 -0700 Subject: [PATCH] Revert "refactor(animations): Ensure async animations applies changes when loaded in zoneless (#55132)" (#55524) This reverts commit 9ab36cfe0af0b5481a1fb9588b0b9db6faaf21fd. PR Close #55524 --- .../src/render/transition_animation_engine.ts | 1 + .../render/transition_animation_engine_spec.ts | 1 + packages/core/src/application/application_ref.ts | 16 +++++++++------- packages/core/src/core_private_export.ts | 2 +- .../test/acceptance/renderer_factory_spec.ts | 6 +++--- .../core/test/render3/change_detection_spec.ts | 6 ++++-- .../async/src/async_animation_renderer.ts | 3 --- .../async/test/animation_renderer_spec.ts | 6 +++++- 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts index f64e5bfa3ce..ed8e3d5aec5 100644 --- a/packages/animations/browser/src/render/transition_animation_engine.ts +++ b/packages/animations/browser/src/render/transition_animation_engine.ts @@ -49,6 +49,7 @@ import { normalizeKeyframes, optimizeGroupPlayer, } from './shared'; +import {NotificationType} from '@angular/core/src/change_detection/scheduling/zoneless_scheduling'; const QUEUED_CLASSNAME = 'ng-animate-queued'; const QUEUED_SELECTOR = '.ng-animate-queued'; diff --git a/packages/animations/browser/test/render/transition_animation_engine_spec.ts b/packages/animations/browser/test/render/transition_animation_engine_spec.ts index de787772d32..2fd1e42f2f5 100644 --- a/packages/animations/browser/test/render/transition_animation_engine_spec.ts +++ b/packages/animations/browser/test/render/transition_animation_engine_spec.ts @@ -57,6 +57,7 @@ const DEFAULT_NAMESPACE_ID = 'id'; getBodyNode(), driver, normalizer || new NoopAnimationStyleNormalizer(), + null, ); engine.createNamespace(DEFAULT_NAMESPACE_ID, element); return engine; diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts index 2d3aed06f02..b3e0da67019 100644 --- a/packages/core/src/application/application_ref.ts +++ b/packages/core/src/application/application_ref.ts @@ -18,7 +18,7 @@ import {inject} from '../di'; import {Injectable} from '../di/injectable'; import {InjectionToken} from '../di/injection_token'; import {Injector} from '../di/injector'; -import {EnvironmentInjector, R3Injector} from '../di/r3_injector'; +import {EnvironmentInjector} from '../di/r3_injector'; import {ErrorHandler, INTERNAL_APPLICATION_ERROR_HANDLER} from '../error_handler'; import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from '../errors'; import {Type} from '../interface/type'; @@ -163,6 +163,9 @@ export interface BootstrapOptions { /** Maximum number of times ApplicationRef will refresh all attached views in a single tick. */ const MAXIMUM_REFRESH_RERUNS = 10; +// This is a temporary type to represent an instance of an R3Injector, which can be destroyed. +// The type will be replaced with a different one once destroyable injector type is available. +type DestroyableInjector = Injector&{destroy?: Function, destroyed?: boolean}; export function _callAndReportToErrorHandler( errorHandler: ErrorHandler, ngZone: NgZone, callback: () => any): any { @@ -549,8 +552,8 @@ export class ApplicationRef { private detectChangesInAttachedViews(refreshViews: boolean) { let rendererFactory: RendererFactory2|null = null; - if (!(this._injector as R3Injector).destroyed) { - rendererFactory = this._injector.get(RendererFactory2); + if (!(this._injector as DestroyableInjector).destroyed) { + rendererFactory = this._injector.get(RendererFactory2, null, {optional: true}); } let runs = 0; @@ -567,7 +570,6 @@ export class ApplicationRef { runs++; afterRenderEffectManager.executeInternalCallbacks(); - // If we have a newly dirty view after running internal callbacks, recheck the views again // before running user-provided callbacks if ([...this.externalTestViews.keys(), ...this._views].some( @@ -575,8 +577,8 @@ export class ApplicationRef { continue; } - // Even if no views require refresh, we need to ensure - // renderFactory begin and end happen to flush animations. + // Flush animations before running afterRender hooks + // This might not have happened if no views were refreshed above rendererFactory?.begin?.(); rendererFactory?.end?.(); @@ -684,7 +686,7 @@ export class ApplicationRef { ngDevMode && 'This instance of the `ApplicationRef` has already been destroyed.'); } - const injector = this._injector as R3Injector; + const injector = this._injector as DestroyableInjector; // Check that this injector instance supports destroy operation. if (injector.destroy && !injector.destroyed) { diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index 1b865bf468b..13d6b595df6 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -11,7 +11,7 @@ export {detectChangesInViewIfRequired as ɵdetectChangesInViewIfRequired, whenSt export {IMAGE_CONFIG as ɵIMAGE_CONFIG, IMAGE_CONFIG_DEFAULTS as ɵIMAGE_CONFIG_DEFAULTS, ImageConfig as ɵImageConfig} from './application/application_tokens'; export {internalCreateApplication as ɵinternalCreateApplication} from './application/create_application'; export {defaultIterableDiffers as ɵdefaultIterableDiffers, defaultKeyValueDiffers as ɵdefaultKeyValueDiffers} from './change_detection/change_detection'; -export {ChangeDetectionScheduler as ɵChangeDetectionScheduler, NotificationType as ɵNotificationType, ZONELESS_ENABLED as ɵZONELESS_ENABLED} from './change_detection/scheduling/zoneless_scheduling'; +export {ChangeDetectionScheduler as ɵChangeDetectionScheduler, ZONELESS_ENABLED as ɵZONELESS_ENABLED} from './change_detection/scheduling/zoneless_scheduling'; export {Console as ɵConsole} from './console'; export {DeferBlockDetails as ɵDeferBlockDetails, getDeferBlocks as ɵgetDeferBlocks} from './defer/discovery'; export {renderDeferBlockState as ɵrenderDeferBlockState, triggerResourceLoading as ɵtriggerResourceLoading} from './defer/instructions'; diff --git a/packages/core/test/acceptance/renderer_factory_spec.ts b/packages/core/test/acceptance/renderer_factory_spec.ts index 61ac7352dd7..4209182d052 100644 --- a/packages/core/test/acceptance/renderer_factory_spec.ts +++ b/packages/core/test/acceptance/renderer_factory_spec.ts @@ -83,20 +83,20 @@ describe('renderer factory lifecycle', () => { it('should work with a component', () => { const fixture = TestBed.createComponent(SomeComponent); - fixture.changeDetectorRef.detectChanges(); + fixture.componentRef.changeDetectorRef.detectChanges(); expect(logs).toEqual([ 'create', 'create', 'begin', 'end', 'begin', 'some_component create', 'some_component update', 'end' ]); logs = []; - fixture.changeDetectorRef.detectChanges(); + fixture.componentRef.changeDetectorRef.detectChanges(); expect(logs).toEqual(['begin', 'some_component update', 'end']); }); it('should work with a component which throws', () => { expect(() => { const fixture = TestBed.createComponent(SomeComponentWhichThrows); - fixture.changeDetectorRef.detectChanges(); + fixture.componentRef.changeDetectorRef.detectChanges(); }).toThrow(); expect(logs).toEqual(['create', 'create', 'begin', 'end', 'begin', 'end']); }); diff --git a/packages/core/test/render3/change_detection_spec.ts b/packages/core/test/render3/change_detection_spec.ts index 021bbd6195a..a5df6ed3417 100644 --- a/packages/core/test/render3/change_detection_spec.ts +++ b/packages/core/test/render3/change_detection_spec.ts @@ -28,11 +28,12 @@ describe('change detection', () => { } } - const fixture = TestBed.createComponent(MyComponent); const rendererFactory = TestBed.inject(RendererFactory2); rendererFactory.begin = () => log.push('begin'); rendererFactory.end = () => log.push('end'); - fixture.changeDetectorRef.detectChanges(); + + const fixture = TestBed.createComponent(MyComponent); + fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).toEqual('works'); @@ -40,6 +41,7 @@ describe('change detection', () => { 'begin', 'detect changes', // regular change detection cycle 'end', + 'detect changes' // check no changes cycle ]); })); }); diff --git a/packages/platform-browser/animations/async/src/async_animation_renderer.ts b/packages/platform-browser/animations/async/src/async_animation_renderer.ts index fac2708736a..2cd098ef815 100644 --- a/packages/platform-browser/animations/async/src/async_animation_renderer.ts +++ b/packages/platform-browser/animations/async/src/async_animation_renderer.ts @@ -22,7 +22,6 @@ import { RendererType2, ɵAnimationRendererType as AnimationRendererType, ɵChangeDetectionScheduler as ChangeDetectionScheduler, - ɵNotificationType as NotificationType, ɵRuntimeError as RuntimeError, } from '@angular/core'; import {ɵRuntimeErrorCode as RuntimeErrorCode} from '@angular/platform-browser'; @@ -128,8 +127,6 @@ export class AsyncAnimationRendererFactory implements OnDestroy, RendererFactory rendererType, ); dynamicRenderer.use(animationRenderer); - // Applying animations might result in new DOM state and should rerun render hooks - this.scheduler?.notify(NotificationType.AfterRenderHooks); }) .catch((e) => { // Permanently use regular renderer when loading fails. diff --git a/packages/platform-browser/animations/async/test/animation_renderer_spec.ts b/packages/platform-browser/animations/async/test/animation_renderer_spec.ts index 0755c0961ca..0fe65678a9e 100644 --- a/packages/platform-browser/animations/async/test/animation_renderer_spec.ts +++ b/packages/platform-browser/animations/async/test/animation_renderer_spec.ts @@ -64,7 +64,11 @@ type AnimationBrowserModule = typeof import('@angular/animations/browser'); engine: MockAnimationEngine, ) => { const animationModule = { - ɵcreateEngine: (_: 'animations' | 'noop', _2: Document): AnimationEngine => engine, + ɵcreateEngine: ( + _: 'animations' | 'noop', + _2: Document, + _3: ChangeDetectionScheduler | null, + ): AnimationEngine => engine, ɵAnimationEngine: MockAnimationEngine as any, ɵAnimationRenderer: AnimationRenderer, ɵBaseAnimationRenderer: BaseAnimationRenderer,