[bug]: event listeners cleanup fail (potential memory leak)

[ernestostifano]

Which react-spring target are you using?

• @react-spring/web
• @react-spring/three
• @react-spring/native
• @react-spring/konva
• @react-spring/zdog

What version of react-spring are you using?

10.0.1

What's Wrong?

When using useScroll event listeners are not being properly cleaned-up when the component unmounts.

I narrowed down the issue to the following two files under @react-spring/shared which are not correctly matching addEventListener with removeEventListener:

• ./src/dom-events/scroll/index.ts
• ./src/dom-events/resize/resizeWindow.ts

Both implementation are ignoring EventListenerOptions while, according to the documentation (see Matching event listeners for removal):

• At least capture option must match across add and remove calls.
• In any case, the recommendation is to use all the same options/args in both calls, because not al browsers behave the same in this regard.

We have automatic tests for event listeners cleanup, and only useScroll is failing in the entire codebase:

Error: UNMATCHED EVENT LISTENERS:
    - SOURCE: window
    - ADD: 4
    - REMOVE: 4
    - UNMATCHED: 4


ADD CALLS:
    - TYPE: resize
    - LISTENER: (REF ID: 0) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: {"passive":true}


    - TYPE: scroll
    - LISTENER: (REF ID: 0) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: {"passive":true}


    - TYPE: resize
    - LISTENER: (REF ID: 1) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: {"passive":true}


    - TYPE: scroll
    - LISTENER: (REF ID: 1) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: {"passive":true}


REMOVE CALLS:
    - TYPE: scroll
    - LISTENER: (REF ID: 0) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: undefined


    - TYPE: resize
    - LISTENER: (REF ID: 0) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: undefined


    - TYPE: scroll
    - LISTENER: (REF ID: 1) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: undefined


    - TYPE: resize
    - LISTENER: (REF ID: 1) () => { containerHandlers?.forEach((handler) => handler.advance()); return true; }
    - OPTIONS: undefined

To Reproduce

Just use useScroll and check for DOM listeners in the inspector. Results might vary depending on browser/environment.

Expected Behaviour

useScroll to properly cleanup event listeners.

Link to repo

N/A.

I would be happy to open a PR to improve/fix this.

[hrishikesh2810]

Hi maintainers 👋
I’d like to work on this issue.
Please let me know if it’s already assigned to someone.
Thanks!