Skip to content

Commit

Permalink
fix(animations): Ensure elements are removed from the cache after lea…
Browse files Browse the repository at this point in the history
…ve animation. (#50929)

This commit fixes a memory leak.

`_namespaceLookup` was cleared before the call to `processLeaveNode()` which was using the lookup.
Without that lookup `clearElementCache()` wasn't called thus keeping a reference to the element.

Fixes #24197 & #50533

PR Close #50929
  • Loading branch information
JeanMeche authored and dylhunn committed Jul 11, 2023
1 parent 5828eb9 commit a14bdfe
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 27 deletions.
Expand Up @@ -508,14 +508,6 @@ export class AnimationTransitionNamespace {
this.players.forEach(p => p.destroy());
this._signalRemovalForInnerTriggers(this.hostElement, context);
}

elementContainsData(element: any): boolean {
let containsData = false;
if (this._elementListeners.has(element)) containsData = true;
containsData =
(this._queue.find(entry => entry.element === element) ? true : false) || containsData;
return containsData;
}
}

export interface QueuedTransition {
Expand Down Expand Up @@ -640,19 +632,18 @@ export class TransitionAnimationEngine {

destroy(namespaceId: string, context: any) {
if (!namespaceId) return;
this.afterFlush(() => {});

const ns = this._fetchNamespace(namespaceId);

this.afterFlush(() => {
this.afterFlushAnimationsDone(() => {
const ns = this._fetchNamespace(namespaceId);
this.namespacesByHostElement.delete(ns.hostElement);
delete this._namespaceLookup[namespaceId];
const index = this._namespaceList.indexOf(ns);
if (index >= 0) {
this._namespaceList.splice(index, 1);
}
ns.destroy(context);
delete this._namespaceLookup[namespaceId];
});

this.afterFlushAnimationsDone(() => ns.destroy(context));
}

private _fetchNamespace(id: string) {
Expand Down Expand Up @@ -1314,16 +1305,6 @@ export class TransitionAnimationEngine {
return rootPlayers;
}

elementContainsData(namespaceId: string, element: any) {
let containsData = false;
const details = element[REMOVAL_FLAG] as ElementAnimationState;
if (details && details.setForRemoval) containsData = true;
if (this.playersByElement.has(element)) containsData = true;
if (this.playersByQueriedElement.has(element)) containsData = true;
if (this.statesByElement.has(element)) containsData = true;
return this._fetchNamespace(namespaceId).elementContainsData(element) || containsData;
}

afterFlush(callback: () => any) {
this._flushFns.push(callback);
}
Expand Down
Expand Up @@ -113,13 +113,20 @@ describe('TransitionAnimationEngine', () => {
registerTrigger(element, engine, trig);
setProperty(element, engine, 'myTrigger', 'value');
engine.flush();
expect(engine.statesByElement.has(element))
.toBe(true, 'Expected element data to be defined.');
expect(engine.playersByElement.has(element))
.toBe(true, 'Expected element data to be defined.');

expect(engine.elementContainsData(DEFAULT_NAMESPACE_ID, element)).toBeTruthy();

engine.destroy(DEFAULT_NAMESPACE_ID, null);
engine.removeNode(DEFAULT_NAMESPACE_ID, element, true);
engine.flush();
engine.players[0].finish();

expect(engine.elementContainsData(DEFAULT_NAMESPACE_ID, element)).toBeTruthy();
expect(engine.statesByElement.has(element))
.toBe(false, 'Expected element data to be undefined.');
expect(engine.playersByElement.has(element))
.toBe(false, 'Expected element data to be undefined.');
});

it('should create and recreate a namespace for a host element with the same component source',
Expand Down

0 comments on commit a14bdfe

Please sign in to comment.