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

Feature: useHydrateAtoms #637

Merged
merged 20 commits into from
Aug 10, 2021
Merged

Feature: useHydrateAtoms #637

merged 20 commits into from
Aug 10, 2021

Conversation

Thisen
Copy link
Collaborator

@Thisen Thisen commented Aug 5, 2021

Adds a new hook, that can hydrate an atom with a initial value.

Primary use case is SSR/Next.js, but it's not bound to anything.

Please supply your use cases, so we can experiment/test this hook properly.

@vercel
Copy link

vercel bot commented Aug 5, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/jotai/7Lr547M83ZgxbPh4ynTY55XsKHxZ
✅ Preview: https://jotai-git-feat-use-hydrate-atoms-pmndrs.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 5, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit baece10:

Sandbox Source
React Configuration
React Typescript Configuration

src/core/contexts.ts Outdated Show resolved Hide resolved
@Nazzanuk
Copy link

Nazzanuk commented Aug 6, 2021

@Thisen looks good I have a couple questions, which I think are essentially the same.

If two components both use useHydrateAtoms will the atom value be initialised once or twice?
If the prop is externally updated, will the atom value be overwritten?

@Thisen
Copy link
Collaborator Author

Thisen commented Aug 6, 2021

@Thisen looks good I have a couple questions, which I think are essentially the same.

If two components both use useHydrateAtoms will the atom value be initialised once or twice?
If the prop is externally updated, will the atom value be overwritten?

The current implementation is:
useHydrateAtoms will run once per use of the hook. Therefore, if you use it multiple places, it will override.
It doesn't care about updates, as it only run once.

Do you see any issues with this?

@dai-shi
Copy link
Member

dai-shi commented Aug 6, 2021

It doesn't care about updates, as it only run once.

Yes. So, for @Nazzanuk 's use case in #340 (comment), useEffect is still required.

@Thisen
Copy link
Collaborator Author

Thisen commented Aug 6, 2021

It doesn't care about updates, as it only run once.

Yes. So, for @Nazzanuk 's use case in #340 (comment), useEffect is still required.

Yup. We might need a better mechanism.

@Nazzanuk
Copy link

Nazzanuk commented Aug 6, 2021

I think it's ok to run only once on the first render, the value will come directly from the atom afterwards anyway, so updating can be handled in a useEffect.

Just thinking about my scenario, I'd only want the atom to be updated once / or maybe only if the value is nullish.

I'd be hesitant to use the current hook in a deeper component with wider reuse as I may not be sure of which instance will render first/last

I could be thinking of a different scenario from what was intended, I still have this in mind.

@dai-shi
Copy link
Member

dai-shi commented Aug 6, 2021

I think it's ok to run only once on the first render,

I also think it's ok. useEffect on user end seems like a valid use case.

I'd be hesitant to use the current hook in a deeper component with wider reuse as I may not be sure of which instance will render first/last

Could you elaborate your concern? I almost understand what this means, but would like to have a better understanding.

@dai-shi
Copy link
Member

dai-shi commented Aug 6, 2021

@Thisen I have one concern. If we restore atoms, atoms onMount won't fire...
(It's the same if we use Provider's initialValues.)

@Nazzanuk
Copy link

Nazzanuk commented Aug 7, 2021

I think this is a bit contrived but it might help illustrate.

const userIdAtom = atom<number | null>(null);

const MyUserWidget = ({ userId }) => {
  useHydrateAtoms([[userIdAtom, userId]]); // what happens if userId is empty?

  const [userId, setUserId] = useAtom(userIdAtom);

  return <>...widget</>;
};

const App = () => <>
  <Header/>
  <Container>
    <MyUserWidget userId={23}/>
    <ChangeUserButton /> {/* Updates userIdAtom */}
    
    <LazyComponent> {/* Rendered lazily or asynchronously */}
      <MyUserWidget userId={23}/>  {/* What should userId be here? Will it reset the first widget just by rendering? */}
    </LazyComponent>
  </Container>
  <Footer/>
</>;

Thinking of an example was quite difficult actually, and this can all be fixed by hoisting useHydrateAtoms to a higher component.

But is there a use case where an atom would need to be hydrated more than once?

@dai-shi
Copy link
Member

dai-shi commented Aug 9, 2021

@Thisen I have one concern. If we restore atoms, atoms onMount won't fire...
(It's the same if we use Provider's initialValues.)

@Thisen My bad. If we use restoreAtoms, the onMount works. Sorry about that.
We should probably prefer the original implementation. btw, you want to work on it after we merge #646.


Oh, onMount would work even with Provider's initialValues. We fixed this probably in #353.

@dai-shi
Copy link
Member

dai-shi commented Aug 9, 2021

But is there a use case where an atom would need to be hydrated more than once?

@Nazzanuk I'm not quite sure how people would like to use it, but useHydrateAtoms would work just once only for atoms that are not mounted.

this can all be fixed by hoisting useHydrateAtoms to a higher component.

I agree with this. My concern is that it's hard to detect misusage from the library, so we would need a good doc about the correct usage.

@Nazzanuk
Copy link

Nazzanuk commented Aug 9, 2021

@Nazzanuk I'm not quite sure how people would like to use it, but useHydrateAtoms would work just once only for atoms that are not mounted.

Ah ok, yeah I think the current implementation works really well. It will make the integration with Next.js much cleaner and more straightforward. If needed I can wrap it in another hook for my particular use case (check first if nullish).

@Thisen Thisen changed the title Experimental feature: useHydrateAtoms Feature: useHydrateAtoms Aug 10, 2021
Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks for your work!

@dai-shi dai-shi merged commit 95cda07 into master Aug 10, 2021
@dai-shi dai-shi deleted the feat/use-hydrate-atoms branch August 10, 2021 09:21
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.

3 participants