Skip to content

Commit

Permalink
fix(vanilla): fix store.ts for an edge case (#2567)
Browse files Browse the repository at this point in the history
* add test for #2365

* does it fix?
  • Loading branch information
dai-shi authored May 27, 2024
1 parent 88362ed commit b74cd46
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 39 deletions.
3 changes: 1 addition & 2 deletions src/vanilla/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ export const createStore = (): Store => {
atomState.d.forEach((_, a) => {
if (!pendingMap.has(a)) {
const aState = getAtomState(a)
pendingStack[pendingStack.length - 1]?.add(a)
pendingMap.set(a, [aState, new Set()])
if (aState) {
addPendingDependent(a, aState)
Expand All @@ -210,8 +209,8 @@ export const createStore = (): Store => {
}
const prevAtomState = getAtomState(atom)
atomStateMap.set(atom, atomState)
pendingStack[pendingStack.length - 1]?.add(atom)
if (!pendingMap.has(atom)) {
pendingStack[pendingStack.length - 1]?.add(atom)
pendingMap.set(atom, [prevAtomState, new Set()])
addPendingDependent(atom, atomState)
}
Expand Down
118 changes: 81 additions & 37 deletions tests/react/dependency.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { StrictMode, Suspense, useEffect, useRef, useState } from 'react'
import { fireEvent, render, waitFor } from '@testing-library/react'
import { render, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { describe, expect, it, vi } from 'vitest'
import { useAtom, useAtomValue, useSetAtom } from 'jotai/react'
import { atom } from 'jotai/vanilla'
Expand Down Expand Up @@ -41,7 +42,7 @@ it('works with 2 level dependencies', async () => {

await findByText('commits: 1, count: 1, doubled: 2, tripled: 6')

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('commits: 2, count: 2, doubled: 4, tripled: 12')
})

Expand Down Expand Up @@ -78,12 +79,12 @@ it('works a primitive atom and a dependent async atom', async () => {
resolve()
await findByText('count: 1, doubled: 2')

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('loading')
resolve()
await findByText('count: 2, doubled: 4')

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('loading')
resolve()
await findByText('count: 3, doubled: 6')
Expand Down Expand Up @@ -138,20 +139,20 @@ it('should keep an atom value even if unmounted', async () => {
})
expect(derivedFn).toHaveReturnedTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 1')
getByText('derived: 1')
})
expect(derivedFn).toHaveReturnedTimes(2)

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await waitFor(() => {
getByText('hidden')
})
expect(derivedFn).toHaveReturnedTimes(2)

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await waitFor(() => {
getByText('count: 1')
getByText('derived: 1')
Expand Down Expand Up @@ -198,15 +199,15 @@ it('should keep a dependent atom value even if unmounted', async () => {
await findByText('derived: 0')
expect(derivedFn).toHaveReturnedTimes(1)

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await findByText('count: 0')
expect(derivedFn).toHaveReturnedTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('count: 1')
expect(derivedFn).toHaveReturnedTimes(1)

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await findByText('derived: 1')
expect(derivedFn).toHaveReturnedTimes(2)
})
Expand Down Expand Up @@ -244,7 +245,7 @@ it('should bail out updating if not changed', async () => {
})
expect(derivedFn).toHaveReturnedTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 0')
getByText('derived: 0')
Expand Down Expand Up @@ -298,7 +299,7 @@ it('should bail out updating if not changed, 2 level', async () => {
expect(getDataObjFn).toHaveReturnedTimes(1)
expect(getAnotherCountFn).toHaveReturnedTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 2')
getByText('anotherCount: 10')
Expand Down Expand Up @@ -340,7 +341,7 @@ it('derived atom to update base atom in callback', async () => {

await findByText('commits: 1, count: 1, doubled: 2')

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('commits: 2, count: 2, doubled: 4')
})

Expand Down Expand Up @@ -370,10 +371,10 @@ it('can read sync derived atom in write without initializing', async () => {

await findByText('count: 1')

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('count: 2')

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('count: 3')
})

Expand Down Expand Up @@ -424,24 +425,24 @@ it('can remount atoms with dependency (#490)', async () => {
getByText('derived: 0')
})

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 1')
getByText('derived: 1')
})

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await waitFor(() => {
getByText('hidden')
})

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await waitFor(() => {
getByText('count: 1')
getByText('derived: 1')
})

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 2')
getByText('derived: 2')
Expand Down Expand Up @@ -505,31 +506,31 @@ it('can remount atoms with intermediate atom', async () => {
getByText('derived: 2')
})

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 2')
getByText('derived: 4')
})

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await waitFor(() => {
getByText('count: 2')
getByText('hidden')
})

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 3')
getByText('hidden')
})

fireEvent.click(getByText('toggle'))
await userEvent.click(getByText('toggle'))
await waitFor(() => {
getByText('count: 3')
getByText('derived: 6')
})

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 4')
getByText('derived: 8')
Expand Down Expand Up @@ -588,7 +589,7 @@ it('can update dependents with useEffect (#512)', async () => {
getByText('derived: 2')
})

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await waitFor(() => {
getByText('count: 2')
getByText('derived: 4')
Expand Down Expand Up @@ -635,11 +636,11 @@ it('update unmounted atom with intermediate atom', async () => {

await findByText('derived: 2')

fireEvent.click(getByText('toggle enabled'))
fireEvent.click(getByText('increment count'))
await userEvent.click(getByText('toggle enabled'))
await userEvent.click(getByText('increment count'))
await findByText('derived: -1')

fireEvent.click(getByText('toggle enabled'))
await userEvent.click(getByText('toggle enabled'))
await findByText('derived: 4')
})

Expand Down Expand Up @@ -681,7 +682,7 @@ it('Should bail for derived sync chains (#877)', async () => {
await findByText('My very long data')
expect(syncAtomCount).toBe(1)

fireEvent.click(getByText(`set value to 'hello'`))
await userEvent.click(getByText(`set value to 'hello'`))

await findByText('My very long data')
expect(syncAtomCount).toBe(1)
Expand Down Expand Up @@ -727,7 +728,7 @@ it('Should bail for derived async chains (#877)', async () => {
await findByText('My very long data')
expect(syncAtomCount).toBe(1)

fireEvent.click(getByText(`set value to 'hello'`))
await userEvent.click(getByText(`set value to 'hello'`))

await findByText('My very long data')
expect(syncAtomCount).toBe(1)
Expand Down Expand Up @@ -772,7 +773,7 @@ it('update correctly with async updates (#1250)', async () => {
getByText('countIsGreaterThanOne: false')
})

fireEvent.click(getByText('Increment Count Twice'))
await userEvent.click(getByText('Increment Count Twice'))
await waitFor(() => {
getByText('alsoCount: 2')
getByText('countIsGreaterThanOne: true')
Expand Down Expand Up @@ -816,7 +817,7 @@ describe('glitch free', () => {
await findByText('value: v0: 0, v1: 0, v2: 0')
expect(computeValue).toHaveBeenCalledTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('value: v0: 1, v1: 1, v2: 1')
expect(computeValue).toHaveBeenCalledTimes(2)
})
Expand Down Expand Up @@ -857,7 +858,7 @@ describe('glitch free', () => {
await findByText('value: 0')
expect(computeValue).toHaveBeenCalledTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('value: 1')
expect(computeValue).toHaveBeenCalledTimes(2)
})
Expand Down Expand Up @@ -900,7 +901,7 @@ describe('glitch free', () => {
await findByText('value: 0')
expect(computeValue).toHaveBeenCalledTimes(1)

fireEvent.click(getByText('button'))
await userEvent.click(getByText('button'))
await findByText('value: 1')
expect(computeValue).toHaveBeenCalledTimes(2)
})
Expand Down Expand Up @@ -950,11 +951,11 @@ it('should not call read function for unmounted atoms in StrictMode (#2076)', as
</StrictMode>,
)

fireEvent.click(getByText('hide'))
await userEvent.click(getByText('hide'))
expect(firstDerivedFn).toBeCalledTimes(1)
firstDerivedFn?.mockClear()

fireEvent.click(getByText('show'))
await userEvent.click(getByText('show'))
expect(firstDerivedFn).toBeCalledTimes(0)
})

Expand Down Expand Up @@ -992,6 +993,49 @@ it('works with unused hook (#2554)', async () => {

await findByText('not running')

fireEvent.click(getByText('Activate'))
await userEvent.click(getByText('Activate'))
await findByText('running')
})

it('works with async dependencies (#2565)', async () => {
const countAtom = atom(0)
const countUpAction = atom(null, (_get, set) => {
set(countAtom, (prev) => prev + 1)
})
const totalCountAtom = atom(async (get) => {
const base = await Promise.resolve(100)
const count = get(countAtom)
return base + count
})

const Count = () => {
const count = useAtomValue(totalCountAtom)
return <p>count: {count}</p>
}
const App = () => {
const up = useSetAtom(countUpAction)
return (
<div>
<button onClick={up}>Count Up</button>
<Suspense fallback="loading">
<Count />
</Suspense>
</div>
)
}

const { getByText, findByText } = render(
<StrictMode>
<App />
</StrictMode>,
)

await findByText('loading')
await findByText('count: 100')

await userEvent.click(getByText('Count Up'))
await findByText('count: 101')

await userEvent.click(getByText('Count Up'))
await findByText('count: 102')
})

0 comments on commit b74cd46

Please sign in to comment.