-
-
Notifications
You must be signed in to change notification settings - Fork 635
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: derived atom is not recomputed after its dependencies changed #2906 #2907
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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. |
commit: |
12fd3bf
to
ed9cfc9
Compare
ed9cfc9
to
8d02414
Compare
Preview in LiveCodesLatest commit: a488c0a
See documentations for usage instructions. |
Size Change: +345 B (+0.37%) Total Size: 92.4 kB
ℹ️ View Unchanged
|
8d02414
to
f7b8170
Compare
src/vanilla/store.ts
Outdated
/** High priority functions */ | ||
[BATCH_PRIORITY_HIGH]: Set<() => void> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing is placed in high priority anymore, but it would be a good place for certain functions.
32b1eb5
to
ba3075a
Compare
Thanks for working on it! I'll have a deeper look later. |
This does seem to fix the issue, but as you might expected it, I'm not a big fan of this change. (And, I didn't like |
d092f6e
to
1d142c0
Compare
1d142c0
to
d4898ee
Compare
Good point. I've refactored the approach so that we are no longer adding props to batch. Please have a second look. Hopefully you like this more. |
d4898ee
to
a3c7ec0
Compare
a3c7ec0
to
a488c0a
Compare
Related Bug Reports or Discussions
#2906
Summary
When a writable atom sets two different atoms, and another atom indirectly depends on both, the dependent atom may fail to update correctly. Specifically, if the first atom's update results in no actual change, the dependent atom's "dirty bit" gets cleared prematurely. Then, when the second
finishRecompute
executes, the dependent atom does not recompute because its dirty bit was already cleared.The proposed solution in this PR is to consolidate all
finishRecompute
operations into a single process, thereby handling changes to the atom graph in one go. This also frees up the high-priority batch channel, making it easier to implement synchronous effects.Check List
pnpm run prettier
for formatting code and docs