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

Serious perf / mem leak in useTracker (WithDeps) >= 2.5.0 #382

Closed
yched opened this issue Feb 1, 2023 · 3 comments · Fixed by #383
Closed

Serious perf / mem leak in useTracker (WithDeps) >= 2.5.0 #382

yched opened this issue Feb 1, 2023 · 3 comments · Fixed by #383

Comments

@yched
Copy link
Contributor

yched commented Feb 1, 2023

Seen with react-meteor-data >= 2.5.0 (2.4 0 is fine), react 17.x or 18.x :
useTracker(fn, deps) updates too frequently, causing performance and memory leaks that accumulate over the component lifetime.

More precisely :
when its reactive dependencies change, then the reactive function reruns N times in a row (each causing a component update)
with N = 1 + [number of times the useTracker deps have changed since the component was mounted]

Simple reproduction code :

import React from 'react';
import { useTracker } from 'meteor/react-meteor-data';
import { ReactiveVar } from 'meteor/reactive-var';

const myVar = new ReactiveVar('A');

export default function TestComponent() {
  const [dep, setDep] = React.useState(0);
  const changeDep = () => setDep(d => d + 1);

  const value = useTracker(() => {
    console.log('re-run');
    return myVar.get();
  }, [dep]);
  const changeValue = () => myVar.set(value === 'A' ? 'B' : 'A');

  return (
    <div>
      Dependency: {dep}<br/>
      Reactive value: {value}<br/>
      <button onClick={changeDep}>Change dependency</button>&nbsp;
      <button onClick={changeValue}>Change reactive value</button>
    </div>
  );
}

"Reactive value" toggles between 'A' and 'B' on each click on "Change reactive value" button
"Dependency" is a counter that increments on each click on "Change dependency"

--> Each click on "Change reactive value" logs the 're-run' message [1 + counter] times

yched referenced this issue Feb 1, 2023
Also re-adds an attempt to retain the "side-effect"
we are creating during first render, when we can,
to both versions of the hook
@Grubba27
Copy link
Contributor

Grubba27 commented Feb 2, 2023

As can be seen in the video below, the issue will be solved in v2.6.2

Thanks a lot for your reproduction it was really helpful

Also thanks a lot @radekmie for the call and the useful links

Screen.Recording.2023-02-02.at.15.54.46.mov

@yched
Copy link
Contributor Author

yched commented Feb 2, 2023

w00t !
Kudos for the super quick fix, glad this could help :-)

@Grubba27
Copy link
Contributor

Grubba27 commented Feb 2, 2023

It was published under the namespace [email protected].
If this issue is still not solved, please reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants