refactor(store): Replace runWithTransactions to flushCallbacks #2946
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: |
|
| Playground | Link |
|---|---|
| React demo | https://livecodes.io?x=id/GLPZBLUSA |
See documentations for usage instructions.
dmaskasky
changed the title
dai-shi
changed the title
| const visited = new WeakSet<AtomState>() | ||
| const stack: AtomState[] = [atomState] | ||
| while (stack.length) { | ||
| const aState = stack.pop()! | ||
| if (!visited.has(aState)) { | ||
| visited.add(aState) | ||
| for (const [d, s] of getMountedOrPendingDependents(aState)) { | ||
| invalidatedAtoms.set(d, s.n) | ||
| stack.push(s) | ||
| if (!invalidatedAtoms.has(d)) { |
There was a problem hiding this comment.
Can you create a test that fails without this condition?
There was a problem hiding this comment.
Its a performance optimization. Transitive, dependents of dependents of A are dependents of A.
There was a problem hiding this comment.
Hm, if it's a performance optimization, it will be difficult to write a test.
|
This looks almost good to me, but for my preference, #2950 is a follow-up PR. |
Summary
Current implementation with
runWithTransactionsuses a single synchronous callback (fn) invocation followed by conditionally running store hooks. For some flows, this code reduces to simply callingfn. The states in which hooks should be called are well defined so refactoring to synchronously calling a finalizer (flushCallbacks) is introduced in this PR.A side-effect of this PR is it resolves several incompatibilities with jotai-effect.
Details
flushCallbacksis called at the end of 7 flows:Check List
pnpm run fix:formatfor formatting code and docs