Skip to content
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

Popper JS memory leak issues #788

Closed
GCHQ-Developer-530 opened this issue Jun 13, 2023 · 1 comment
Closed

Popper JS memory leak issues #788

GCHQ-Developer-530 opened this issue Jun 13, 2023 · 1 comment
Assignees
Labels
component: ic-button The generic component, includes both the web component and the React component component: ic-tooltip The generic component, includes both the web component and the React component
Milestone

Comments

@GCHQ-Developer-530
Copy link
Contributor

GCHQ-Developer-530 commented Jun 13, 2023

Summary of the bug

Popper JS is causing memory leak issues.

🪜 How to reproduce

Popper is adding scroll and resize event listeners to the window. The event handler triggers a Popper forceUpdate on scroll which is expensive and blocks the scroll which is the reason for the performance issue. The Stencil issue we thought it was (#682) compounds the Popper issue as none of these event listeners are being removed.

Looks like someone has spotted a memory leak issue in the tutorial and we've followed that. floating-ui/popper.js.org#47 This has been tried and doesn't fix our memory leak issues by itself, but will probably still need changing.

Could either be updating how we add and remove event listeners, or we may need to look into converting to floating ui which could turn out to be a lot of work.

🧐 Expected behaviour

No performance issues.

@ad9242
Copy link
Contributor

ad9242 commented Jun 13, 2023

May be possible to turn off the scroll listener? https://popper.js.org/docs/v2/modifiers/event-listeners/

Or at very least it should not add the event listeners if disableTooltip is set to true. At the moment that prop only determines the visibility

@ad9242 ad9242 self-assigned this Jun 13, 2023
@ad9242 ad9242 moved this to Dev In Progress in Intelligence Community Design System Jun 13, 2023
@ad9242 ad9242 added component: ic-button The generic component, includes both the web component and the React component component: ic-tooltip The generic component, includes both the web component and the React component labels Jun 13, 2023
@ad9242 ad9242 added this to the V2.x milestone Jun 13, 2023
@ad9242 ad9242 moved this from Dev In Progress to In Review in Intelligence Community Design System Jun 14, 2023
@ASM995 ASM995 closed this as completed Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ic-button The generic component, includes both the web component and the React component component: ic-tooltip The generic component, includes both the web component and the React component
Projects
Development

No branches or pull requests

3 participants