GetJobber / atlantis

🔱 Atlantis
https://atlantis.getjobber.com
MIT License
27 stars 30 forks source link

refactor(hooks): Add useOnMount and useCallbackRef [96563] #2037

Closed Aiden-Brine closed 1 month ago

Aiden-Brine commented 1 month ago

Motivations

There are many copies of useOnMount and useIsMounted throughout the codebase. useOnMount performs a callback one time once the component is mounted whereas useIsMounted returns a ref that stores a boolean on whether the component is mounted or not. I think combining them would violate the Single Responsibility Principle because providing information about if the component is mounted and performing actions once the component is mounted are two different responsibilities.

Unfortunately, there are many cases where useIsMounted is used when useOnMount would be preferable. For example here and here the logic could be simplified by using useOnMount. That being said there are occasions like here and here where useIsMounted is required. For this reason we need both the current functionality of useIsMounted and useOnMount but we only need one version of each available in Atlantis that can be used in the other repos. There will be a followup PRs to replace the versions of these two hooks in the other repos with the Atlantis versions. useOnMount also uses a hook useCallbackRef which will also be pulled into Atlantis and is currently only ever used by useOnMount.

The only difference between the two current instances of useIsMounted (atlantis and jobber-mobile) is the fact that Atlantis uses useSafeLayoutEffect instead of useLayoutEffect which was added as a part of Remix. I chatted about this with Scott and he suggested that even though we are backing away from Remix it would still be good to use useSafeLayoutEffect.

Changes

Added

Adds useCallbackRef and useOnMount. You can see these are identical to those used in other projects:

Changed

No existing functionality is changed

Deprecated

Removed

Fixed

Security

Testing

I added useOnMount to Storybook. I also tested that the Atlantis onMount will work via pre-release on this this branch and for a little added testing I loaded one of the components with some console logging to verify that onMount is only called once despite the component being rendered multiple times:

Screenshot 2024-09-24 at 11 03 04 AM

Changes can be tested via Pre-release


In Atlantis we use Github's built in pull request reviews.

Random photo of Atlantis

cloudflare-workers-and-pages[bot] commented 1 month ago

Deploying atlantis with  Cloudflare Pages  Cloudflare Pages

Latest commit: d3a3837
Status: âœ…  Deploy successful!
Preview URL: https://358a841a.atlantis.pages.dev
Branch Preview URL: https://job-96563-add-use-on-mounted.atlantis.pages.dev

View logs

github-actions[bot] commented 1 month ago

Published Pre-release for 047c24dccaaa3c3d089e6f57a2eb571259151405 with versions:

  - @jobber/components@5.35.1-JOB-96563-047c24d.4+047c24dc
  - @jobber/hooks@2.11.1-JOB-96563-047c24d.43+047c24dc

To install the new version(s) for Web run:

npm install @jobber/components@5.35.1-JOB-96563-047c24d.4+047c24dc @jobber/hooks@2.11.1-JOB-96563-047c24d.43+047c24dc

To install the new version(s) for Mobile run:

npm install @jobber/hooks@2.11.1-JOB-96563-047c24d.43+047c24dc