Skip to content
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

alternate approach for #2907 #2919

Draft
wants to merge 32 commits into
base: fix-derived-recompute-not-changing
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6908f9c
add failing test
dmaskasky Jan 4, 2025
ade5a39
a single recomputeDependents in flushBatch
dmaskasky Jan 6, 2025
c985ca8
centralize finishRecompute
dmaskasky Jan 6, 2025
ba3075a
cleanup
dmaskasky Jan 6, 2025
3e0e324
fix test
dmaskasky Jan 6, 2025
7a66fd0
simplify test
dmaskasky Jan 6, 2025
c6e4d81
remove redundant changedAtoms set
dmaskasky Jan 6, 2025
ea0ed26
mark original atom changed
dmaskasky Jan 6, 2025
3b435d7
flushBatch while condition includes dependents size
dmaskasky Jan 6, 2025
04734ef
Merge branch 'main' into fix-derived-recompute-not-changing
dai-shi Jan 7, 2025
aecaa09
Merge branch 'main' into fix-derived-recompute-not-changing-2
dai-shi Jan 7, 2025
d8b73b5
recover the test
dai-shi Jan 7, 2025
ccf0fec
Merge branch 'fix-derived-recompute-not-changing' into fix-derived-re…
dai-shi Jan 7, 2025
8f5a036
Merge branch 'main' into fix-derived-recompute-not-changing-2
dai-shi Jan 7, 2025
e5a43d9
add failing test
dmaskasky Jan 4, 2025
15a1f13
a single recomputeDependents in flushBatch
dmaskasky Jan 6, 2025
bbfaee5
centralize finishRecompute
dmaskasky Jan 6, 2025
27ce275
cleanup
dmaskasky Jan 6, 2025
cca2b18
fix test
dmaskasky Jan 6, 2025
6a892f5
simplify test
dmaskasky Jan 6, 2025
c920946
remove redundant changedAtoms set
dmaskasky Jan 6, 2025
048359a
mark original atom changed
dmaskasky Jan 6, 2025
ba84b18
flushBatch while condition includes dependents size
dmaskasky Jan 6, 2025
a488c0a
refactor: move flushBatch in buildStore and call recomputeDependents …
dmaskasky Jan 7, 2025
8347d62
Merge commit 'e5a43d98219367f02291f4dd64ba94566135a440' into fix-deri…
dai-shi Jan 7, 2025
ef52454
remove log
dai-shi Jan 7, 2025
113f4bc
Merge branch 'main' into fix-derived-recompute-not-changing-2
dai-shi Jan 7, 2025
0245b72
Merge branch 'main' into fix-derived-recompute-not-changing
dai-shi Jan 7, 2025
13f159e
Merge branch 'fix-derived-recompute-not-changing' into fix-derived-re…
dai-shi Jan 7, 2025
5c503d3
use map for batch
dai-shi Jan 7, 2025
c08c09c
make dirty flag a number
dai-shi Jan 7, 2025
628f569
Revert "use map for batch"
dai-shi Jan 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/vanilla/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ type AtomState<Value = AnyValue> = {
/** Atom error */
e?: AnyError
/** Indicates that the atom value has been changed */
x?: true
x: number
}

const isAtomStateInitialized = <Value>(atomState: AtomState<Value>) =>
Expand Down Expand Up @@ -290,7 +290,7 @@ const buildStore = (...storeArgs: StoreArgs): Store => {
}
let atomState = getAtomState(atom)
if (!atomState) {
atomState = { d: new Map(), p: new Set(), n: 0 }
atomState = { d: new Map(), p: new Set(), n: 0, x: 0 }
setAtomState(atom, atomState)
atomOnInit?.(atom, store)
}
Expand All @@ -315,7 +315,6 @@ const buildStore = (...storeArgs: StoreArgs): Store => {
atomState.v = valueOrPromise
}
delete atomState.e
delete atomState.x
if (!hasPrevValue || !Object.is(prevValue, atomState.v)) {
++atomState.n
if (pendingPromise) {
Expand Down Expand Up @@ -428,7 +427,6 @@ const buildStore = (...storeArgs: StoreArgs): Store => {
} catch (error) {
delete atomState.v
atomState.e = error
delete atomState.x
++atomState.n
return atomState
} finally {
Expand Down Expand Up @@ -493,7 +491,7 @@ const buildStore = (...storeArgs: StoreArgs): Store => {
// Atom has been visited but not yet processed
visited.add(a)
// Mark atom dirty
aState.x = true
++aState.x
stack.pop()
continue
}
Expand Down Expand Up @@ -527,7 +525,7 @@ const buildStore = (...storeArgs: StoreArgs): Store => {
changedAtoms.add(a)
}
}
delete aState.x
--aState.x
}
}
addBatchFunc(batch, 0, finishRecompute)
Expand Down
67 changes: 66 additions & 1 deletion tests/vanilla/store.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,6 @@ it('recomputes dependents of unmounted atoms', () => {
const a = atom(0)
a.debugLabel = 'a'
const bRead = vi.fn((get: Getter) => {
console.log('bRead')
return get(a)
})
const b = atom(bRead)
Expand All @@ -1108,3 +1107,69 @@ it('recomputes dependents of unmounted atoms', () => {
store.set(w)
expect(bRead).not.toHaveBeenCalled()
})

it('recomputes all changed atom dependents together', async () => {
const a = atom([0])
const b = atom([0])
const a0 = atom((get) => get(a)[0]!)
const b0 = atom((get) => get(b)[0]!)
const a0b0 = atom((get) => [get(a0), get(b0)])
const w = atom(null, (_, set) => {
set(a, [0])
set(b, [1])
})
const store = createStore()
store.sub(a0b0, () => {})
store.set(w)
expect(store.get(a0)).toBe(0)
expect(store.get(b0)).toBe(1)
expect(store.get(a0b0)).toEqual([0, 1])
})

it('runs recomputeDependents on atoms in the correct order', async () => {
const store = createStore()
let i = 0
function createHistoryAtoms<T>(initialValue: T) {
const historyStackAtom = atom<T[]>([initialValue])
historyStackAtom.debugLabel = `${i}:historyStackAtom`
const historyIndexAtom = atom(0)
historyIndexAtom.debugLabel = `${i}:historyIndexAtom`
const valueAtom = atom(
(get) => get(historyStackAtom)[get(historyIndexAtom)]!,
)
valueAtom.debugLabel = `${i}:valueAtom`
const resetAtom = atom(null, (_, set, value: T) => {
set(historyStackAtom, [value])
set(historyIndexAtom, 0)
})
resetAtom.debugLabel = `${i}:resetAtom`
i++
return { valueAtom, resetAtom }
}
const val1Atoms = createHistoryAtoms('foo')
const val2Atoms = createHistoryAtoms<string | null>(null)
const initAtom = atom(null, (_get, set) => {
// if comment out this line, the test will pass
set(val2Atoms.resetAtom, null)
set(val1Atoms.resetAtom, 'bar')
})
initAtom.debugLabel = 'initAtom'
const computedValAtom = atom((get) => {
const v2Value = get(val2Atoms.valueAtom)
if (v2Value !== null) {
return v2Value
}
const v1Value = get(val1Atoms.valueAtom)
return v1Value
})
computedValAtom.debugLabel = 'computedValAtom'
const asyncInitAtom = atom(null, async (_get, set) => {
// if comment out this line, the test will pass [DOES NOT WORK]
await new Promise((resolve) => setTimeout(resolve, 0))
set(initAtom)
})
store.sub(computedValAtom, () => {})
await store.set(asyncInitAtom)
const result = store.get(computedValAtom)
expect(result).toBe('bar')
})
Loading