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

hydra-dispatch has a memory leaking issue #14

Open
okupara opened this issue Nov 25, 2019 · 3 comments
Open

hydra-dispatch has a memory leaking issue #14

okupara opened this issue Nov 25, 2019 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@okupara
Copy link

okupara commented Nov 25, 2019

hydra-dispatch-react has a memory leaking issue because it internally updates state and developers have no way to cancel it.

The steps to reproduce is below.

  1. fire dispatch for an async request when component mount or render
  2. move to any paths before the request which sends at (1.) gets finished
  3. react complains about memory leaking or an unnecessary rendering happens that it occurs memory leaking.

I created a test app to reproduce it.
https://codesandbox.io/s/eager-bhaskara-g6h08

There are two components they do dispatching the same dummy async request with Promise. "Test1" is using hydra-dispatch-react and "Test2" is using just only react-hooks.

When you push the "with hydra-dispatch" button, and then wait for seconds, you can see memory leak warning.

ml

On the other hand, after you reload the test app(because that warning shows up only one time), and push the "with hooks" button, and then wait for seconds, nothing happens, it means memory leak doesn't happen.

especially on developing SPA(applications have some url paths), this problem could annoy developers.

I think hydra-dispatch-react should have an option to cancel updating state.

@okupara okupara added the bug Something isn't working label Nov 25, 2019
@chaabaj
Copy link
Contributor

chaabaj commented Nov 25, 2019

Ok, I see, but the cancellation with react setState is done by checking the ref. I guess we could make aware dispatcherFromReact from the component reference and check it in the scheduler. It will do the same as Test2.tsx.

My guess is if you remove the code about checking the ref in Test2.tsx you would have the same issue.

@okupara
Copy link
Author

okupara commented Nov 25, 2019

I see.

My guess is if you remove the code about checking the ref in Test2.tsx you would have the same issue.

Of course it does, my point is hydra-dispatch-react currently doesn't have any way to deal with that situation, but hooks does.

@chaabaj
Copy link
Contributor

chaabaj commented Nov 25, 2019

Yes so we need to allow dispatcherFromReact to track the reference

@chaabaj chaabaj mentioned this issue Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants