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

Fix useDidUpdate to make it work with Strict Effects #974

Merged
merged 2 commits into from
May 20, 2022

Conversation

som-sm
Copy link
Contributor

@som-sm som-sm commented May 2, 2022

With the release of React 18, StrictMode gets an additional behavior to ensure it's compatible with reusable state. When StrictMode is enabled, React intentionally double-invokes effects (mount -> unmount -> mount) for newly mounted components. Source

So, it's essential that the hasMountedRef is reset when unmounting otherwise it won't behave desirably when wrapped inside StrictMode.

som-sm added 2 commits May 2, 2022 16:15
> With the release of React 18, StrictMode gets an additional behavior to ensure it's compatible with reusable state. When StrictMode is enabled, **React intentionally double-invokes effects (mount -> unmount -> mount) for newly mounted components**. [Source](reactwg/react-18#19)
So, it's essential that the `hasMountedRef` is reset when unmounting otherwise it won't behave desirably when wrapped inside StrictMode.
@som-sm som-sm changed the title Add cleanup to make it work with Strict Effects Fix useDidUpdate to make it work with Strict Effects May 2, 2022
@som-sm som-sm closed this May 2, 2022
@som-sm som-sm reopened this May 2, 2022
@imbhargav5
Copy link
Owner

Wow. Thanks for this! Will review this tonight!

@som-sm
Copy link
Contributor Author

som-sm commented May 7, 2022

@imbhargav5 Hey, did you manage to review this?

@imbhargav5
Copy link
Owner

Hi @ssmkhrj I haven't had much of a chance to test the docs and be sure of approving this. What would really help me is if we added some tests to show the exact behaviour like some of our other hooks. Could I bother you to add a few tests for this ? That would help immensely.

@codecov
Copy link

codecov bot commented May 10, 2022

Codecov Report

Merging #974 (1885c04) into main (f31b76f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #974      +/-   ##
==========================================
+ Coverage   91.09%   91.11%   +0.01%     
==========================================
  Files          67       67              
  Lines        4022     4028       +6     
  Branches      665      665              
==========================================
+ Hits         3664     3670       +6     
  Misses        357      357              
  Partials        1        1              
Impacted Files Coverage Δ
src/hooks/useDidUpdate.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f31b76f...1885c04. Read the comment docs.

@imbhargav5 imbhargav5 merged commit 7e64d10 into imbhargav5:main May 20, 2022
@github-actions
Copy link
Contributor

🎉 This PR is included in version 5.11.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants