-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug(CdkVirtualScrollViewport): no unsubscribe from scrolling after destroy #27799
Labels
Comments
crisbeto
added a commit
to crisbeto/material2
that referenced
this issue
Sep 18, 2023
… destroyed Initially the virtual scroll viewport was written under the assumption that it is the scrollable and that it doesn't need to unsubscribe from its `elementScrolled` stream, because it'll be completed on destroy. This might not be the case if a `CdkVirtualScrollableElement` is provided and the viewport might be destroyed without the scrollable being destroyed. These changes add a `takeUntil` to avoid any potential leaks. Fixes angular#27799.
crisbeto
added a commit
that referenced
this issue
Sep 18, 2023
… destroyed (#27800) Initially the virtual scroll viewport was written under the assumption that it is the scrollable and that it doesn't need to unsubscribe from its `elementScrolled` stream, because it'll be completed on destroy. This might not be the case if a `CdkVirtualScrollableElement` is provided and the viewport might be destroyed without the scrollable being destroyed. These changes add a `takeUntil` to avoid any potential leaks. Fixes #27799. (cherry picked from commit 31187ab)
crisbeto
added a commit
that referenced
this issue
Sep 18, 2023
… destroyed (#27800) Initially the virtual scroll viewport was written under the assumption that it is the scrollable and that it doesn't need to unsubscribe from its `elementScrolled` stream, because it'll be completed on destroy. This might not be the case if a `CdkVirtualScrollableElement` is provided and the viewport might be destroyed without the scrollable being destroyed. These changes add a `takeUntil` to avoid any potential leaks. Fixes #27799.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
Is this a regression?
The previous version in which this bug was not present was
No response
Description
CdkVirtualScrollViewport
subscribes tothis.scrollable.elementScrolled()
but does not unsubscribe from it. In most cases, when you are not usingcdkVirtualScrollingElement
, this will not be any problem becausescrollable
unsubscribes when destroyed, but when we usecdkVirtualScrollingElement
and thecdk-virtual-scroll-viewport
element is mounted/unmounted from the DOM, there will be unnecessary scroll subscriptions. This subscriptions can lead to some unexpected behaviours.Reproduction
StackBlitz link: https://stackblitz.com/edit/l1quzt-cwdqhc?file=src%2Fexample%2Fcdk-virtual-scroll-parent-scrolling-example.html
Steps to reproduce:
FixedSizeVirtualScrollStrategy.prototype.onContentScrolled
(vendor.js 22253 line).Expected Behavior
After small scroll the
onContentScrolled
should be called only one time withthis._viewport
inFixedSizeVirtualScrollStrategy
that equalCdkVirtualScrollViewport
instance.Actual Behavior
After small scroll the
FixedSizeVirtualScrollStrategy.prototype.onContentScrolled
will be called a lot of time withthis._viewport === null
inFixedSizeVirtualScrollStrategy
.Environment
The text was updated successfully, but these errors were encountered: