From c57ce8f038b238952ddfb0833b6e5263233a6a81 Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Tue, 7 Jan 2025 22:10:54 -0800 Subject: [PATCH 1/6] test: should update dependents with the value of the unwrapped atom when the promise resolves --- tests/vanilla/utils/unwrap.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/vanilla/utils/unwrap.test.ts b/tests/vanilla/utils/unwrap.test.ts index be294dde54..26e0dbcc4a 100644 --- a/tests/vanilla/utils/unwrap.test.ts +++ b/tests/vanilla/utils/unwrap.test.ts @@ -150,3 +150,17 @@ describe('unwrap', () => { expect(store.get(syncAtom)).toEqual('concrete') }) }) + +it('should update dependents with the value of the unwrapped atom when the promise resolves', async () => { + const store = createStore() + const asyncTarget = atom(() => Promise.resolve('value')) + const target = unwrap(asyncTarget) + const results: string[] = [] + const derived = atom(async (get) => { + await Promise.resolve() + results.push('effect ' + get(target)) + }) + store.sub(derived, () => {}) + await new Promise((r) => setTimeout(r)) + expect(results).toEqual(['effect undefined', 'effect value']) +}) From 4da8912a5e76414efe2cecb8cf9f072046066f56 Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Wed, 8 Jan 2025 01:19:18 -0800 Subject: [PATCH 2/6] handle mounting and unmounting of sync and async dependencies separately --- src/vanilla/store.ts | 12 +++-- tests/vanilla/dependency.test.tsx | 86 +++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index 53f9e40156..77cf67978c 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -75,6 +75,8 @@ type Mounted = { readonly l: Set<() => void> /** Set of mounted atoms that the atom depends on. */ readonly d: Set + /** Set of dependencies added asynchronously */ + readonly q: Set /** Set of mounted atoms that depends on the atom. */ readonly t: Set /** Function to run when the atom is unmounted. */ @@ -303,10 +305,8 @@ const buildStore = (...storeArgs: StoreArgs): Store => { for (const a of atomState.d.keys()) { addPendingPromiseToDependency(atom, valueOrPromise, ensureAtomState(a)) } - atomState.v = valueOrPromise - } else { - atomState.v = valueOrPromise } + atomState.v = valueOrPromise delete atomState.e if (!hasPrevValue || !Object.is(prevValue, atomState.v)) { ++atomState.n @@ -404,6 +404,11 @@ const buildStore = (...storeArgs: StoreArgs): Store => { const valueOrPromise = atomRead(atom, getter, options as never) setAtomStateValueOrPromise(atom, atomState, valueOrPromise) if (isPromiseLike(valueOrPromise)) { + if (batch) { + addBatchFunc(batch, 0, () => atomState.m?.q.clear()) + } else { + atomState.m?.q.clear() + } valueOrPromise.onCancel?.(() => controller?.abort()) valueOrPromise.then(mountDependenciesIfAsync, mountDependenciesIfAsync) } @@ -599,6 +604,7 @@ const buildStore = (...storeArgs: StoreArgs): Store => { atomState.m = { l: new Set(), d: new Set(atomState.d.keys()), + q: new Set(), t: new Set(), } atomState.h?.() diff --git a/tests/vanilla/dependency.test.tsx b/tests/vanilla/dependency.test.tsx index 90f0c114e7..6369a0329b 100644 --- a/tests/vanilla/dependency.test.tsx +++ b/tests/vanilla/dependency.test.tsx @@ -425,3 +425,89 @@ it('batches sync writes', () => { expect(fetch).toBeCalledWith(1) expect(store.get(a)).toBe(1) }) + +it('mounts and unmounts sync and async dependencies correctly', async () => { + const mounted = [0, 0, 0, 0, 0] as [number, number, number, number, number] + const a = atom(0) + a.onMount = () => { + ++mounted[0] + return () => { + --mounted[0] + } + } + + const b = atom(0) + b.onMount = () => { + ++mounted[1] + return () => { + --mounted[1] + } + } + + const c = atom(0) + c.onMount = () => { + ++mounted[2] + return () => { + --mounted[2] + } + } + + const d = atom(0) + d.onMount = () => { + ++mounted[3] + return () => { + --mounted[3] + } + } + + const e = atom(0) + e.onMount = () => { + ++mounted[4] + return () => { + --mounted[4] + } + } + + let resolve: (() => Promise) | undefined + const f = atom((get) => { + if (!get(a)) { + get(b) + } else { + get(c) + } + const promise = new Promise((r) => { + resolve = () => { + r() + return promise + } + }).then(() => { + if (!get(a)) { + get(d) + } else { + get(e) + } + }) + return promise + }) + + const store = createStore() + // mount a, b synchronously + const unsub = store.sub(f, () => {}) + expect(mounted).toEqual([1, 1, 0, 0, 0]) + + // mount d asynchronously + await resolve!() + expect(mounted).toEqual([1, 1, 0, 1, 0]) + + // unmount b, mount c synchronously + store.set(a, 1) + expect(mounted).toEqual([1, 0, 1, 1, 0]) + + // unmount d, mount e asynchronously + await resolve!() + expect(mounted).toEqual([1, 0, 1, 0, 1]) + + // unmount a, b, d, e synchronously + unsub() + expect(mounted).toEqual([0, 0, 0, 0, 0]) +}) From 555738f9eaaf9cb7336070a50127b07d28c0c5d3 Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Tue, 14 Jan 2025 22:27:51 -0800 Subject: [PATCH 3/6] add failing tests --- tests/testUtils.ts | 24 ++++++++++++++++++++++++ tests/vanilla/dependency.test.tsx | 9 ++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 tests/testUtils.ts diff --git a/tests/testUtils.ts b/tests/testUtils.ts new file mode 100644 index 0000000000..d0520dd12e --- /dev/null +++ b/tests/testUtils.ts @@ -0,0 +1,24 @@ +import { createStore } from 'jotai' + +type Store = ReturnType +type GetAtomState = Parameters[0]>[0] +type DebugStore = Store & { getAtomState: GetAtomState } + +export function createDebugStore() { + let getAtomState: GetAtomState + const store = createStore().unstable_derive((...storeArgs) => { + ;[getAtomState] = storeArgs + const [, setAtomState] = storeArgs + storeArgs[1] = (atom, atomState) => { + return setAtomState( + atom, + Object.assign(atomState, { label: atom.debugLabel }), + ) + } + return storeArgs + }) + if (getAtomState! === undefined) { + throw new Error('failed to create debug store') + } + return Object.assign(store, { getAtomState }) as DebugStore +} diff --git a/tests/vanilla/dependency.test.tsx b/tests/vanilla/dependency.test.tsx index 6369a0329b..0512e9fde3 100644 --- a/tests/vanilla/dependency.test.tsx +++ b/tests/vanilla/dependency.test.tsx @@ -1,5 +1,6 @@ import { expect, it, vi } from 'vitest' import { atom, createStore } from 'jotai/vanilla' +import { createDebugStore } from '../../tests/testUtils' it('can propagate updates with async atom chains', async () => { const store = createStore() @@ -429,6 +430,7 @@ it('batches sync writes', () => { it('mounts and unmounts sync and async dependencies correctly', async () => { const mounted = [0, 0, 0, 0, 0] as [number, number, number, number, number] const a = atom(0) + a.debugLabel = 'a' a.onMount = () => { ++mounted[0] return () => { @@ -437,6 +439,7 @@ it('mounts and unmounts sync and async dependencies correctly', async () => { } const b = atom(0) + b.debugLabel = 'b' b.onMount = () => { ++mounted[1] return () => { @@ -445,6 +448,7 @@ it('mounts and unmounts sync and async dependencies correctly', async () => { } const c = atom(0) + c.debugLabel = 'c' c.onMount = () => { ++mounted[2] return () => { @@ -453,6 +457,7 @@ it('mounts and unmounts sync and async dependencies correctly', async () => { } const d = atom(0) + d.debugLabel = 'd' d.onMount = () => { ++mounted[3] return () => { @@ -461,6 +466,7 @@ it('mounts and unmounts sync and async dependencies correctly', async () => { } const e = atom(0) + e.debugLabel = 'e' e.onMount = () => { ++mounted[4] return () => { @@ -489,8 +495,9 @@ it('mounts and unmounts sync and async dependencies correctly', async () => { }) return promise }) + f.debugLabel = 'f' - const store = createStore() + const store = createDebugStore() // mount a, b synchronously const unsub = store.sub(f, () => {}) expect(mounted).toEqual([1, 1, 0, 0, 0]) From a1e19649c542a8ea6dbc8a0e98c3ce95dfe07419 Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Tue, 14 Jan 2025 22:30:05 -0800 Subject: [PATCH 4/6] drop store changes --- src/vanilla/store.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index 77cf67978c..53f9e40156 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -75,8 +75,6 @@ type Mounted = { readonly l: Set<() => void> /** Set of mounted atoms that the atom depends on. */ readonly d: Set - /** Set of dependencies added asynchronously */ - readonly q: Set /** Set of mounted atoms that depends on the atom. */ readonly t: Set /** Function to run when the atom is unmounted. */ @@ -305,8 +303,10 @@ const buildStore = (...storeArgs: StoreArgs): Store => { for (const a of atomState.d.keys()) { addPendingPromiseToDependency(atom, valueOrPromise, ensureAtomState(a)) } + atomState.v = valueOrPromise + } else { + atomState.v = valueOrPromise } - atomState.v = valueOrPromise delete atomState.e if (!hasPrevValue || !Object.is(prevValue, atomState.v)) { ++atomState.n @@ -404,11 +404,6 @@ const buildStore = (...storeArgs: StoreArgs): Store => { const valueOrPromise = atomRead(atom, getter, options as never) setAtomStateValueOrPromise(atom, atomState, valueOrPromise) if (isPromiseLike(valueOrPromise)) { - if (batch) { - addBatchFunc(batch, 0, () => atomState.m?.q.clear()) - } else { - atomState.m?.q.clear() - } valueOrPromise.onCancel?.(() => controller?.abort()) valueOrPromise.then(mountDependenciesIfAsync, mountDependenciesIfAsync) } @@ -604,7 +599,6 @@ const buildStore = (...storeArgs: StoreArgs): Store => { atomState.m = { l: new Set(), d: new Set(atomState.d.keys()), - q: new Set(), t: new Set(), } atomState.h?.() From 538a4fcd2fedf6bccdbe52aae794d839f205ad67 Mon Sep 17 00:00:00 2001 From: daishi Date: Thu, 16 Jan 2025 22:44:56 +0900 Subject: [PATCH 5/6] alternate fix --- src/vanilla/store.ts | 10 +++- tests/testUtils.ts | 24 -------- tests/vanilla/dependency.test.tsx | 93 ------------------------------- 3 files changed, 8 insertions(+), 119 deletions(-) delete mode 100644 tests/testUtils.ts diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index 53f9e40156..5a4023e08b 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -566,11 +566,17 @@ const buildStore = (...storeArgs: StoreArgs): Store => { const mountDependencies = (atom: AnyAtom, atomState: AtomState) => { if (atomState.m && !isPendingPromise(atomState.v)) { - for (const a of atomState.d.keys()) { + for (const [a, n] of atomState.d) { if (!atomState.m.d.has(a)) { - const aMounted = mountAtom(a, ensureAtomState(a)) + const aState = ensureAtomState(a) + const aMounted = mountAtom(a, aState) aMounted.t.add(atom) atomState.m.d.add(a) + if (n !== aState.n) { + changedAtoms.set(a, aState) + aState.u?.() + invalidateDependents(aState) + } } } for (const a of atomState.m.d || []) { diff --git a/tests/testUtils.ts b/tests/testUtils.ts deleted file mode 100644 index d0520dd12e..0000000000 --- a/tests/testUtils.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { createStore } from 'jotai' - -type Store = ReturnType -type GetAtomState = Parameters[0]>[0] -type DebugStore = Store & { getAtomState: GetAtomState } - -export function createDebugStore() { - let getAtomState: GetAtomState - const store = createStore().unstable_derive((...storeArgs) => { - ;[getAtomState] = storeArgs - const [, setAtomState] = storeArgs - storeArgs[1] = (atom, atomState) => { - return setAtomState( - atom, - Object.assign(atomState, { label: atom.debugLabel }), - ) - } - return storeArgs - }) - if (getAtomState! === undefined) { - throw new Error('failed to create debug store') - } - return Object.assign(store, { getAtomState }) as DebugStore -} diff --git a/tests/vanilla/dependency.test.tsx b/tests/vanilla/dependency.test.tsx index 0512e9fde3..90f0c114e7 100644 --- a/tests/vanilla/dependency.test.tsx +++ b/tests/vanilla/dependency.test.tsx @@ -1,6 +1,5 @@ import { expect, it, vi } from 'vitest' import { atom, createStore } from 'jotai/vanilla' -import { createDebugStore } from '../../tests/testUtils' it('can propagate updates with async atom chains', async () => { const store = createStore() @@ -426,95 +425,3 @@ it('batches sync writes', () => { expect(fetch).toBeCalledWith(1) expect(store.get(a)).toBe(1) }) - -it('mounts and unmounts sync and async dependencies correctly', async () => { - const mounted = [0, 0, 0, 0, 0] as [number, number, number, number, number] - const a = atom(0) - a.debugLabel = 'a' - a.onMount = () => { - ++mounted[0] - return () => { - --mounted[0] - } - } - - const b = atom(0) - b.debugLabel = 'b' - b.onMount = () => { - ++mounted[1] - return () => { - --mounted[1] - } - } - - const c = atom(0) - c.debugLabel = 'c' - c.onMount = () => { - ++mounted[2] - return () => { - --mounted[2] - } - } - - const d = atom(0) - d.debugLabel = 'd' - d.onMount = () => { - ++mounted[3] - return () => { - --mounted[3] - } - } - - const e = atom(0) - e.debugLabel = 'e' - e.onMount = () => { - ++mounted[4] - return () => { - --mounted[4] - } - } - - let resolve: (() => Promise) | undefined - const f = atom((get) => { - if (!get(a)) { - get(b) - } else { - get(c) - } - const promise = new Promise((r) => { - resolve = () => { - r() - return promise - } - }).then(() => { - if (!get(a)) { - get(d) - } else { - get(e) - } - }) - return promise - }) - f.debugLabel = 'f' - - const store = createDebugStore() - // mount a, b synchronously - const unsub = store.sub(f, () => {}) - expect(mounted).toEqual([1, 1, 0, 0, 0]) - - // mount d asynchronously - await resolve!() - expect(mounted).toEqual([1, 1, 0, 1, 0]) - - // unmount b, mount c synchronously - store.set(a, 1) - expect(mounted).toEqual([1, 0, 1, 1, 0]) - - // unmount d, mount e asynchronously - await resolve!() - expect(mounted).toEqual([1, 0, 1, 0, 1]) - - // unmount a, b, d, e synchronously - unsub() - expect(mounted).toEqual([0, 0, 0, 0, 0]) -}) From 7e9d1250f97c9fa1675a6722900973f264414497 Mon Sep 17 00:00:00 2001 From: daishi Date: Mon, 20 Jan 2025 22:00:30 +0900 Subject: [PATCH 6/6] follow store refactor --- src/vanilla/store.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index d78de2a47a..a5eb556dc7 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -332,6 +332,7 @@ const buildStore = (...storeArgs: StoreArgs): Store => { const mountDependenciesIfAsync = () => { if (atomState.m) { mountDependencies(atom, atomState) + recomputeInvalidatedAtoms() flushCallbacks() } }