components
components copied to clipboard
bug(ListKeyManager): ListKeyManager contain rxjs subscription memory leaks
Is this a regression?
- [ ] Yes, this behavior used to work in the previous version
The previous version in which this bug was not present was
No response
Description
There are a couple of observable subscriptions, but the mechanism of unsubscribing (using takeUntil or .unsubscribe) isn't implemented when we don't need that class anymore. This behavior could cause memory leaks in an app.
For example in withTypeAhead method, there is a call of this._typeaheadSubscription.unsubscribe();, but it works only for the previous subscription when call this method couple of times:
- some component calls for the first time
withTypeAhead(), a subscription is created - when it calls it for the second time, the subscription from the previous step is unsubscribed, and subscribed one more time
- after the component is destroyed, the last subscription will still be in the memory
Also, the same subscription issue is in the constructor https://github.com/angular/components/blob/8504c7761f4fb334753e8ec4d8624ad85f071444/src/cdk/a11y/key-manager/list-key-manager.ts#L68
Reproduction
Steps to reproduce:
- Open src/cdk/a11y/key-manager/list-key-manager.ts
- In the method
withTypeAheadafterthis._typeaheadSubscription.unsubscribe();add some logging (console.log('LIST KEY MANAGER subscribed')) - Pass the third argument to
subscribefunction like() => console.log('LISTKEYMANAGER unsubscribed') - Run any test which uses
ListKeyManager, for examplearn test src/cdk/a11y --debugoryarn test src/cdk/menu --debug
After adding logs withTypeAhead could look like this:
withTypeAhead(debounceInterval: number = 200): this {
if (
(typeof ngDevMode === 'undefined' || ngDevMode) &&
this._items.length &&
this._items.some(item => typeof item.getLabel !== 'function')
) {
throw Error('ListKeyManager items in typeahead mode must implement the `getLabel` method.');
}
this._typeaheadSubscription.unsubscribe();
console.log('LIST KEY MANAGER subscribed');
// Debounce the presses of non-navigational keys, collect the ones that correspond to letters
// and convert those letters back into a string. Afterwards find the first item that starts
// with that string and select it.
this._typeaheadSubscription = this._letterKeyStream
.pipe(
tap(letter => this._pressedLetters.push(letter)),
debounceTime(debounceInterval),
filter(() => this._pressedLetters.length > 0),
map(() => this._pressedLetters.join('')),
)
.subscribe(
inputString => {
const items = this._getItemsArray();
// Start at 1 because we want to start searching at the item immediately
// following the current active item.
for (let i = 1; i < items.length + 1; i++) {
const index = (this._activeItemIndex + i) % items.length;
const item = items[index];
if (
!this._skipPredicateFn(item) &&
item.getLabel!().toUpperCase().trim().indexOf(inputString) === 0
) {
this.setActiveItem(index);
break;
}
}
this._pressedLetters = [];
},
undefined,
() => console.log('LIST KEY MANAGER unsubscribed'),
);
return this;
}
Expected Behavior
'LIST KEY MANAGER unsubscribed' string should be in the console exactly the same times as 'LIST KEY MANAGER subscribed' string.
Actual Behavior
Only 'LIST KEY MANAGER subscribed' string is presented in console.
Environment
- Angular:
- CDK/Material: https://github.com/angular/components/commit/e63390c712b8e66b81b6f026dcfd313c0fb3a682
- Browser(s): —
- Operating System (e.g. Windows, macOS, Ubuntu): MacOS 12.5