From 1953ca40ddd031174afba21b6fe042ffd5365836 Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Wed, 13 Nov 2024 15:53:14 -0800 Subject: [PATCH 1/4] call onmount flushPending in finally block --- src/vanilla/store.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index 223e96d1d9..2a568dc348 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -584,11 +584,13 @@ const buildStore = ( let isSync = true try { const onUnmount = atomOnMount(atom, (...args) => { - const result = writeAtomState(pending, atom, ...args) - if (!isSync) { - flushPending(pending) + try { + return writeAtomState(pending, atom, ...args) + } finally { + if (!isSync) { + flushPending(pending) + } } - return result }) if (onUnmount) { mounted.u = () => { From 059bd8ded02ee00b5c57c93d47830c028917de8e Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Wed, 13 Nov 2024 16:05:32 -0800 Subject: [PATCH 2/4] temp remove unmount test cases. to be fixed later with pending injection (#2810) --- tests/vanilla/store.test.tsx | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/vanilla/store.test.tsx b/tests/vanilla/store.test.tsx index 92401f2431..73a72e9e61 100644 --- a/tests/vanilla/store.test.tsx +++ b/tests/vanilla/store.test.tsx @@ -621,11 +621,6 @@ describe('should invoke flushPending only after all atoms are updated (#2804)', mountResult.push('before onMount setSelf') setSelf(1) mountResult.push('after onMount setSelf') - return () => { - mountResult.push('before onUnmount setSelf') - setSelf(2) - mountResult.push('after onUnmount setSelf') - } } mountResult.push('before store.sub') store.sub(a, () => { @@ -633,20 +628,12 @@ describe('should invoke flushPending only after all atoms are updated (#2804)', }) const unsub = store.sub(m, () => {}) mountResult.push('after store.sub') - mountResult.push('before store.unsub') - unsub() - mountResult.push('after store.unsub') expect(mountResult).not.toEqual([ 'before store.sub', 'before onMount setSelf', 'a value changed - 1', 'after onMount setSelf', 'after store.sub', - 'before store.unsub', - 'before onUnmount setSelf', - 'a value changed - 2', - 'after onUnmount setSelf', - 'after store.unsub', ]) expect(mountResult).toEqual([ 'before store.sub', @@ -654,11 +641,6 @@ describe('should invoke flushPending only after all atoms are updated (#2804)', 'after onMount setSelf', 'a value changed - 1', 'after store.sub', - 'before store.unsub', - 'before onUnmount setSelf', - 'after onUnmount setSelf', - 'a value changed - 2', - 'after store.unsub', ]) }) }) From 598518d81f27ffaed66e58eedb3ac1c29c1aa3f8 Mon Sep 17 00:00:00 2001 From: David Maskasky Date: Wed, 13 Nov 2024 17:05:50 -0800 Subject: [PATCH 3/4] flushpending everywhere --- src/vanilla/store.ts | 78 ++++++++++++++++++++---------------- tests/vanilla/store.test.tsx | 2 + 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index 2a568dc348..af2949453f 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -331,17 +331,21 @@ const buildStore = ( } return returnAtomValue(aState) } - // a !== atom - const aState = readAtomState(pending, a, dirtyAtoms) - if (isSync) { - addDependency(pending, atom, atomState, a, aState) - } else { - const pending = createPending() - addDependency(pending, atom, atomState, a, aState) - mountDependencies(pending, atom, atomState) - flushPending(pending) + let aState: AtomState + try { + // a !== atom + aState = readAtomState(pending, a, dirtyAtoms) + return returnAtomValue(aState) + } finally { + if (isSync) { + addDependency(pending, atom, atomState, a, aState!) + } else { + const pending = createPending() + addDependency(pending, atom, atomState, a, aState!) + mountDependencies(pending, atom, atomState) + flushPending(pending) + } } - return returnAtomValue(aState) } let controller: AbortController | undefined let setSelf: ((...args: unknown[]) => unknown) | undefined @@ -492,29 +496,32 @@ const buildStore = ( a: WritableAtom, ...args: As ) => { - const aState = getAtomState(a) - let r: R | undefined - if (isSelfAtom(atom, a)) { - if (!hasInitialValue(a)) { - // NOTE technically possible but restricted as it may cause bugs - throw new Error('atom not writable') + let aState: AtomState + aState = getAtomState(a) + try { + if (isSelfAtom(atom, a)) { + if (!hasInitialValue(a)) { + // NOTE technically possible but restricted as it may cause bugs + throw new Error('atom not writable') + } + const hasPrevValue = 'v' in aState + const prevValue = aState.v + const v = args[0] as V + setAtomStateValueOrPromise(a, aState, v) + mountDependencies(pending, a, aState) + if (!hasPrevValue || !Object.is(prevValue, aState.v)) { + addPendingAtom(pending, a, aState) + recomputeDependents(pending, a, aState) + } + return undefined as R + } else { + return writeAtomState(pending, a, ...args) } - const hasPrevValue = 'v' in aState - const prevValue = aState.v - const v = args[0] as V - setAtomStateValueOrPromise(a, aState, v) - mountDependencies(pending, a, aState) - if (!hasPrevValue || !Object.is(prevValue, aState.v)) { - addPendingAtom(pending, a, aState) - recomputeDependents(pending, a, aState) + } finally { + if (!isSync) { + flushPending(pending) } - } else { - r = writeAtomState(pending, a, ...args) as R - } - if (!isSync) { - flushPending(pending) } - return r as R } try { return atomWrite(atom, getter, setter, ...args) @@ -528,9 +535,11 @@ const buildStore = ( ...args: Args ): Result => { const pending = createPending() - const result = writeAtomState(pending, atom, ...args) - flushPending(pending) - return result + try { + return writeAtomState(pending, atom, ...args) + } finally { + flushPending(pending) + } } const mountDependencies = ( @@ -643,7 +652,8 @@ const buildStore = ( const subscribeAtom = (atom: AnyAtom, listener: () => void) => { const pending = createPending() const atomState = getAtomState(atom) - const mounted = mountAtom(pending, atom, atomState) + let mounted: Mounted + mounted = mountAtom(pending, atom, atomState) flushPending(pending) const listeners = mounted.l listeners.add(listener) diff --git a/tests/vanilla/store.test.tsx b/tests/vanilla/store.test.tsx index 73a72e9e61..5a1e80798a 100644 --- a/tests/vanilla/store.test.tsx +++ b/tests/vanilla/store.test.tsx @@ -582,6 +582,7 @@ it('should update derived atom even if dependances changed (#2697)', () => { describe('should invoke flushPending only after all atoms are updated (#2804)', () => { const store = createStore() + it('should invoke flushPending only after all atoms are updated with set', () => { const a = atom(0) const setResult = [] @@ -611,6 +612,7 @@ describe('should invoke flushPending only after all atoms are updated (#2804)', 'after store.set', ]) }) + it('should invoke flushPending only after all atoms are updated with mount', () => { const mountResult = [] const a = atom(0) From 968c52d4aeac72068cc185e4feb4ef2a42167b08 Mon Sep 17 00:00:00 2001 From: Daishi Kato Date: Thu, 14 Nov 2024 11:02:43 +0900 Subject: [PATCH 4/4] Apply suggestions from code review --- src/vanilla/store.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/vanilla/store.ts b/src/vanilla/store.ts index af2949453f..aa805753f5 100644 --- a/src/vanilla/store.ts +++ b/src/vanilla/store.ts @@ -331,10 +331,9 @@ const buildStore = ( } return returnAtomValue(aState) } - let aState: AtomState + // a !== atom + const aState = readAtomState(pending, a, dirtyAtoms) try { - // a !== atom - aState = readAtomState(pending, a, dirtyAtoms) return returnAtomValue(aState) } finally { if (isSync) { @@ -496,8 +495,7 @@ const buildStore = ( a: WritableAtom, ...args: As ) => { - let aState: AtomState - aState = getAtomState(a) + const aState = getAtomState(a) try { if (isSelfAtom(atom, a)) { if (!hasInitialValue(a)) { @@ -652,8 +650,7 @@ const buildStore = ( const subscribeAtom = (atom: AnyAtom, listener: () => void) => { const pending = createPending() const atomState = getAtomState(atom) - let mounted: Mounted - mounted = mountAtom(pending, atom, atomState) + const mounted = mountAtom(pending, atom, atomState) flushPending(pending) const listeners = mounted.l listeners.add(listener)