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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid state update when component is unmount #55

Merged
merged 3 commits into from
May 4, 2020

Conversation

Dot-H
Copy link
Collaborator

@Dot-H Dot-H commented Apr 19, 2020

What

Hi 馃憢, this PR closes #54 and avoid unnecessary renders.

How

It replaces the useEffectAsync and fetchURL with a new useMountFetch hook and moves the link-related state management in the hook.

See this commit for the implementation of the hook.

useEffectAsync

This hook keeps the request state (response, loading, error) and wraps the call to ScraperWraper so that:

  • The state is not updated if the component is unmounted
  • The request is cancelled if the component is unmounted

Moreover, this hooks has a loading state set to true as default to avoid unnecessary render on mount (due to setting the loading state to true).

ReactTinyLink

Uses the useEffectAsync hook return values to manage its state. It passes a default value to the hook's response so that in case of error, the value of the component does not change and there is potentially less re-render inside the whole component (I did not profile, this is just an assumption).

After this PR, the components renders only two times. Against four before.

  • The initial render
  • The render when loading is done (error or success)

Extra

I took the liberty to add an example in the demo to show the removal of the link. If you want me to this can be removed 馃檪

Of course I am open to the discussion and do not hesitate to ask me for changes. Thank you for your time !

Alexandre Bernard added 3 commits April 19, 2020 14:34
Signed-off-by: Alexandre Bernard <alexandre.bernard@epita.fr>
Signed-off-by: Alexandre Bernard <alexandre.bernard@epita.fr>
Signed-off-by: Alexandre Bernard <alexandre.bernard@epita.fr>
@Dot-H Dot-H changed the title Fix memory leak Avoid state update when component is unmount Apr 19, 2020
@Dot-H
Copy link
Collaborator Author

Dot-H commented May 3, 2020

Hi @winhtaikaung, any interest on this ? If not, do you mind if I make an open source project based on this repo just exposing the hook ?

@winhtaikaung
Copy link
Owner

Hi @winhtaikaung, any interest on this ? If not, do you mind if I make an open source project based on this repo just exposing the hook ?

Hi, @Dot-H sorry for late response, I ve looked through it and looks good to me will merge it and release it by tonight. I would love to invite you to this project as contributor if you dont mind.

@winhtaikaung winhtaikaung merged commit ebed038 into winhtaikaung:master May 4, 2020
@Dot-H
Copy link
Collaborator Author

Dot-H commented May 4, 2020

Hi @winhtaikaung, no worries. Thanks for the review, I'd be please to be a contributor 馃檪

@winhtaikaung
Copy link
Owner

@all-contributors please add @Dot-H for bug and code

@allcontributors
Copy link
Contributor

@winhtaikaung

I've put up a pull request to add @Dot-H! 馃帀

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 this pull request may close these issues.

Memory leak when component unmount before fetch done
2 participants