cds-icon subscribes to state updates observable GlobalStateService.stateUpdates in its firstUpdated callback.
The unsubscribe logic for GlobalStateService.stateUpdates is placed in the disconnectCallback.
The memory leak happens because it's not guaranteed that disconnectCallback will be called after firstUpdated in terms of callback execution order. There are situations where disconnectCallback is called first and then firstUpdated is called. When this happens and disconnectCallback is called first, the cds-icon will 'leak' subscribers to the GlobalStateService.stateUpdates observable. These leaked subscribers are never unsubscribed and this leads to the memory leak.
The reason why disconnectCallback is sometimes called before firstUpdated is related to the way LitElement.requestUpdated() works - this is the code https://github.com/lit/lit/blob/93d671feab82688a79fc60ba22cf204fa4ca02ec/packages/reactive-element/src/reactive-element.ts#L1150.
In short, requestUpdated() works asynchronously which means the firstUpdated callback will be called asynchronously. Since disconnectCallback works synchronously there are situations where firstUpdated will be called after disconnectCallback.
How to reproduce
To reproduce the problem, you can run unit test spec file using Angular 13 with testing module teardown.destroyAfterEach enabled (teardown is enabled by default in Angular 13).
In the spec below, every it will leak 1 subscriber to the GlobalStateService.stateUpdates, that's why the expectations (expect) in the consecutive it(...) are for 1, 2, 3 and 4 subscribers respectively.
NOTE: if you run the spec file, make sure the spec randomized execution is disabled in your karma.conf.js. Also, you must use Angular 13 with teardown enabled. By doing this, the disconnectCallback will be called before firstUpdated consistently. In Angular 13 projects (with teardown enabled) that have a lot of unit tests, these leaked subscribers cause significant test slowdown as the test execution progresses.
SPEC FILE CODE
import { Component, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { ClarityIcons, popOutIcon } from '@cds/core/icon';
import { CdsModule } from '@cds/angular';
import { GlobalStateService } from '@cds/core/internal';
ClarityIcons.addIcons(popOutIcon);
@Component({
selector: 'test-component',
template: `
<cds-icon shape="pop-out"></cds-icon>
`
})
export class TestComponent {
}
describe('Test GlobalStateService.stateUpdates.subscriptions Memory Leak', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [
CdsModule
],
declarations: [
TestComponent
],
schemas: [
CUSTOM_ELEMENTS_SCHEMA
],
teardown: {
// With Angular 13 'destroyAfterEach' is enabled by default.
destroyAfterEach: true
}
})
});
it('create icon for the first time - currently there are no subscribers in the global service', () => {
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(0);
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
// At this point the 'cds-icon' element is added to the DOM and element's connectedCallback() has been called.
// But there are still no subscribers in the global service because CdsIcon.firstUpdated() callback is not called yet.
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(0);
});
it('verify 1 subscriber is currently leaked in the global service', () => {
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(1);
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(1);
});
it('verify 2 subscribers are currently leaked in the global service', () => {
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(2);
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(2);
});
it('verify 3 subscribers are currently leaked in the global service', () => {
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(3);
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
expect(((GlobalStateService.stateUpdates as any).subscriptions as any[]).length).toEqual(3);
});
});
How to reproduce in production code
The easiest way to reproduce the issue is with the following code:
const icon = document.createElement('cds-icon');
icon.setAttribute('shape', 'pop-out');
document.body.appendChild(icon);
icon.remove();Creating an icon element, attaching it to the DOM and then immediately removing it will result in leaked subscribers.
Here is a stackblitz that uses the code above to leak 1000 subscriptions https://stackblitz.com/edit/clarity-dark-theme-clr13-cds6-duukxq?file=src/app/app.component.html
Expected behavior
Adding and removing cds-icon from the DOM should not leak subscribers to GlobalStateService.stateUpdates.
It seems firstUpdated() callback is not the place where cds-icon (and possibly other components from Clarity Core) should subscribe to GlobalStateService.stateUpdates.
Probably, the connectedCallback() is the place where the subscribe logic should be placed.
Versions
Clarity project:
Clarity version:
Framework:
Framework version:
Angular 13
Device:
- Type: MacBook
- OS: MacOS Monterey
- Browser: Chrome
- Version: 100.0.4896.127
Contains the Web Components that work in any JavaScript framework.
Contains shims for core usage in Angular environment
Contains shims for core usage in React environment
Our open source sans-serif typeface.
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent