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: should update dependents with the value of the unwrapped atom when the promise resolves #2936

Merged
merged 9 commits into from
Jan 20, 2025

Conversation

dmaskasky
Copy link
Collaborator

@dmaskasky dmaskasky commented Jan 15, 2025

Related Bug Reports or Discussions

Fix #2914

Summary

Caused by 34b4f8e and e777f46

It's caused by a subtle timing issue. The promise is resolved before the derived atom is mounted because the mount process runs after the promise is resolved.

There might be a better fix, but for now in this PR, it triggers updates again if it finds atom value change after the mount process. More refactors for store.ts are coming and we may revisit this in the future.

Check List

  • pnpm run fix:format for formatting code and docs

Copy link

vercel bot commented Jan 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jotai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 20, 2025 1:01pm

@dmaskasky dmaskasky changed the title Fix unwrap atom effect 1 fix: should update dependents with the value of the unwrapped atom when the promise resolves Jan 15, 2025
@dmaskasky dmaskasky requested a review from dai-shi January 15, 2025 06:40
Copy link

codesandbox-ci bot commented Jan 15, 2025

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.

Copy link

github-actions bot commented Jan 15, 2025

LiveCodes Preview in LiveCodes

Latest commit: 7e9d125
Last updated: Jan 20, 2025 1:00pm (UTC)

Playground Link
React demo https://livecodes.io?x=id/9E4362EZ4

See documentations for usage instructions.

Copy link

pkg-pr-new bot commented Jan 15, 2025

Open in Stackblitz

More templates

npm i https://pkg.pr.new/jotai@2936

commit: 7e9d125

@dai-shi
Copy link
Member

dai-shi commented Jan 15, 2025

Thanks for cleaning it up. I'll have a closer look later.

Caused by 34b4f8e and e777f46

Is this caused by the combination of them? It doesn't fail with only one of them, does it?

@dai-shi
Copy link
Member

dai-shi commented Jan 15, 2025

This is such a fundamental thing... I'll take it over. I'll push some commits to #2920.

@dai-shi dai-shi marked this pull request as draft January 16, 2025 13:06
@dai-shi dai-shi added this to the v2.11.1 milestone Jan 16, 2025
@dai-shi dai-shi marked this pull request as ready for review January 16, 2025 23:19
@dai-shi dai-shi merged commit 3c51426 into pmndrs:main Jan 20, 2025
44 checks passed
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.

2 participants