Skip to content

Commit

Permalink
fix(store2): "stuck" derived atoms where some derivations may never r…
Browse files Browse the repository at this point in the history
…esolve (#2448)

* failing test case for never resolving derivations

* skip one test

* wip: use epoch number to compare

* revert for the write case

---------

Co-authored-by: daishi <daishi@axlight.com>
  • Loading branch information
backbone87 and dai-shi authored May 27, 2024
1 parent fca316f commit 0a5a16b
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/vanilla/store2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ export const createStore = (): Store => {
if (pendingPromise) {
if (pendingPromise !== valueOrPromise) {
pendingPromise[CONTINUE_PROMISE](valueOrPromise, abortPromise)
++atomState.n
}
} else {
const continuablePromise = createContinuablePromise(
Expand Down Expand Up @@ -499,8 +500,7 @@ export const createStore = (): Store => {
for (let i = topsortedAtoms.length - 1; i >= 0; --i) {
const a = topsortedAtoms[i]!
const aState = getAtomState(a)
const hasPrevValue = 'v' in aState
const prevValue = aState.v
const prevEpochNumber = aState.n
let hasChangedDeps = false
for (const dep of aState.d.keys()) {
if (dep !== a && changedAtoms.has(dep)) {
Expand All @@ -511,7 +511,7 @@ export const createStore = (): Store => {
if (hasChangedDeps) {
readAtomState(pending, a, isMarked)
mountDependencies(pending, a, aState)
if (!hasPrevValue || !Object.is(prevValue, aState.v)) {
if (prevEpochNumber !== aState.n) {
addPendingAtom(pending, a, aState)
changedAtoms.add(a)
}
Expand Down
82 changes: 82 additions & 0 deletions tests/vanilla/dependency.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,85 @@ it('should not provide stale values to conditional dependents', () => {
store.set(hasFilterAtom, true)
expect(store.get(stageAtom), 'should update').toBe('is-empty')
})

it('settles never resolving async derivations with deps picked up sync', async () => {
const resolve: ((value: number) => void)[] = []

const syncAtom = atom({
promise: new Promise<number>((r) => resolve.push(r)),
})

const asyncAtom = atom(async (get) => {
return await get(syncAtom).promise
})

const store = createStore()

let sub = 0
const values: unknown[] = []
store.get(asyncAtom).then((value) => values.push(value))

store.sub(asyncAtom, () => {
sub++
store.get(asyncAtom).then((value) => values.push(value))
})

await new Promise((r) => setTimeout(r))

store.set(syncAtom, {
promise: new Promise<number>((r) => resolve.push(r)),
})

await new Promise((r) => setTimeout(r))

resolve[1]?.(1)

await new Promise((r) => setTimeout(r))

expect(values).toEqual([1, 1])
expect(sub).toBe(1)
})

it.skipIf(!import.meta.env?.USE_STORE2)(
'settles never resolving async derivations with deps picked up async',
async () => {
const resolve: ((value: number) => void)[] = []

const syncAtom = atom({
promise: new Promise<number>((r) => resolve.push(r)),
})

const asyncAtom = atom(async (get) => {
// we want to pick up `syncAtom` as an async dep
await Promise.resolve()

return await get(syncAtom).promise
})

const store = createStore()

let sub = 0
const values: unknown[] = []
store.get(asyncAtom).then((value) => values.push(value))

store.sub(asyncAtom, () => {
sub++
store.get(asyncAtom).then((value) => values.push(value))
})

await new Promise((r) => setTimeout(r))

store.set(syncAtom, {
promise: new Promise<number>((r) => resolve.push(r)),
})

await new Promise((r) => setTimeout(r))

resolve[1]?.(1)

await new Promise((r) => setTimeout(r))

expect(values).toEqual([1, 1])
expect(sub).toBe(1)
},
)

0 comments on commit 0a5a16b

Please sign in to comment.