Skip to content

Commit

Permalink
fix remaining tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@dmaskasky
dmaskasky committed Jan 21, 2025
1 parent 030508d commit d751b83
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 101 deletions.
4 changes: 4 additions & 0 deletions src/vanilla/store.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ type SecretStoreMethods = readonly [
) => void,
makeGetter: <Value>(atom: Atom<Value>, isSync?: boolean) => Getter,
makeSetter: <Value>(atom: Atom<Value>, isSync?: boolean) => Setter,
invalidateDependents: <Value>(atom: Atom<Value>) => void,
recomputeInvalidatedAtoms: () => void,
flushCallbacks: () => void,
]

type Store = {
Expand Down Expand Up @@ -737,7 +739,9 @@ const buildStore: BuildStore = (
mountDependencies,
makeGetter,
makeSetter,
invalidateDependents,
recomputeInvalidatedAtoms,
flushCallbacks,
],
}
return store
Expand Down
126 changes: 52 additions & 74 deletions tests/syncEffect/syncEffect.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
import React, { createElement, useEffect } from 'react'
import { afterEach, beforeEach } from 'node:test'
import { act, render, renderHook, waitFor } from '@testing-library/react'
import { createElement } from 'react'
import { act, render, waitFor } from '@testing-library/react'
import { describe, expect, it, vi } from 'vitest'
import { Provider, useAtom, useAtomValue, useSetAtom } from 'jotai/react'
import { Provider, useAtomValue } from 'jotai/react'
import { atom } from 'jotai/vanilla'
import { syncEffect } from './syncEffect'
import {
ErrorBoundary,
assert,
createDebugStore,
delay,
incrementLetter,
} from './test-utils'
import { assert, createDebugStore, delay, incrementLetter } from './test-utils'

it('should run the effect on vanilla store', function test() {
const store = createDebugStore()
Expand Down Expand Up @@ -707,13 +700,13 @@ describe('synchronous updates to the same atom', () => {
}
})

it('should not batch effect setStates', async function test() {
it('should batch effect setStates', async function test() {
const valueAtom = atom(0)
valueAtom.debugLabel = 'valueAtom'

const runCount = { current: 0 }
let runCount = 0
const derivedAtom = atom((get) => {
++runCount.current
++runCount
return get(valueAtom)
})
derivedAtom.debugLabel = 'derivedAtom'
Expand Down Expand Up @@ -743,11 +736,11 @@ it('should not batch effect setStates', async function test() {
store.sub(effectAtom, () => {})

expect(store.get(valueAtom)).toBe(0)
expect(runCount.current).toBe(1)
expect(runCount).toBe(1)

store.set(triggerAtom, (v) => !v)
expect(store.get(valueAtom)).toBe(2)
expect(runCount.current).toBe(3) // <--- not batched (we would expect runCount to be 2 if batched)
expect(runCount).toBe(2) // <--- batched (we would expect runCount to be 3 if not batched)
})

it('should batch synchronous updates as a single transaction', function test() {
Expand Down Expand Up @@ -959,22 +952,31 @@ it('should not infinite loop with nested atomEffects', async function test() {
countAtom.debugLabel = 'countAtom'
countAtom.onMount = () => {
++metrics.mounted
return () => ++metrics.unmounted
return () => {
++metrics.unmounted
}
}

let delayedIncrement = false
const effectAtom = syncEffect((_get, set) => {
let resolve: () => Promise<void>
const effect1Atom = syncEffect((_get, set) => {
++metrics.runCount1
if (metrics.runCount1 > 1) throw new Error('infinite loop')
Promise.resolve().then(() => {
delayedIncrement = true
if (metrics.runCount1 > 1) {
throw new Error('infinite loop')
}
const promise = new Promise<void>((r) => {
resolve = () => {
r()
return promise
}
})
promise.then(() => {
set(countAtom, (v) => v + 1)
})
})
effectAtom.debugLabel = 'effectAtom'
effect1Atom.debugLabel = 'effect1Atom'

const readOnlyAtom = atom((get) => {
get(effectAtom)
get(effect1Atom)
return get(countAtom)
})
readOnlyAtom.debugLabel = 'readOnlyAtom'
Expand All @@ -987,17 +989,23 @@ it('should not infinite loop with nested atomEffects', async function test() {

const store = createDebugStore()

const { internalAtom, refAtom } = effectAtom as any
const { internalAtom, refAtom } = effect1Atom as any
const countAtomState = store.getAtomState(countAtom)
const effectAtomState = store.getAtomState(effectAtom)
const effectAtomState = store.getAtomState(effect1Atom)
const internalAtomState = store.getAtomState(internalAtom)
const refAtomState = store.getAtomState(refAtom)
const readOnlyAtomState = store.getAtomState(readOnlyAtom)
const effect2AtomState = store.getAtomState(effect2Atom)

store.sub(effect2Atom, () => {})
expect(metrics).toEqual({
mounted: 1,
runCount1: 1,
runCount2: 1,
unmounted: 0,
})

await waitFor(() => assert(delayedIncrement))
await resolve!()

expect(metrics).toEqual({
mounted: 1,
Expand Down Expand Up @@ -1031,41 +1039,33 @@ it('should not rerun with get.peek', function test() {
expect(runCount).toBe(1)
})

it('should trigger the error boundary when an error is thrown', async function test() {
const effectAtom = syncEffect((_get, _set) => {
throw new Error('effect error')
it('should throw on set when an error is thrown in effect', async function test() {
const refreshAtom = atom(0)
refreshAtom.debugLabel = 'refresh'

const effectAtom = syncEffect((get) => {
if (get(refreshAtom) === 1) {
throw new Error('effect error')
}
})
effectAtom.debugLabel = 'effect'

const store = createDebugStore()

const { internalAtom, refAtom } = effectAtom as any
const refreshAtomState = store.getAtomState(refreshAtom)
const effectAtomState = store.getAtomState(effectAtom)
const internalAtomState = store.getAtomState(internalAtom)
const refAtomState = store.getAtomState(refAtom)

function TestComponent() {
useAtomValue(effectAtom, { store })
return <div>test</div>
}
store.sub(effectAtom, () => {})

let didThrow = false
const originalConsoleError = console.error
try {
console.error = vi.fn()
render(
<ErrorBoundary componentDidCatch={() => (didThrow = true)}>
<TestComponent />
</ErrorBoundary>,
)
} finally {
console.error = originalConsoleError
}
await waitFor(() => assert(didThrow))
expect(didThrow).toBe(true)
expect(() => {
store.set(refreshAtom, (v) => v + 1)
}).toThrowError('effect error')
})

it('should trigger an error boundary when an error is thrown in a cleanup', async function test() {
it('should throw on set when an error is thrown in cleanup', async function test() {
const refreshAtom = atom(0)
refreshAtom.debugLabel = 'refresh'

Expand All @@ -1085,33 +1085,11 @@ it('should trigger an error boundary when an error is thrown in a cleanup', asyn
const internalAtomState = store.getAtomState(internalAtom)
const refAtomState = store.getAtomState(refAtom)

function TestComponent() {
useAtomValue(effectAtom, { store })
return <div>test</div>
}
store.sub(effectAtom, () => {})

let didThrow = false
render(
<ErrorBoundary
componentDidCatch={(error, _errorInfo) => {
if (!didThrow) {
expect(error.message).toBe('effect cleanup error')
}
didThrow = true
}}
>
<TestComponent />
</ErrorBoundary>,
expect(() => store.set(refreshAtom, (v) => v + 1)).toThrowError(
'effect cleanup error',
)

const originalConsoleError = console.error
try {
console.error = vi.fn()
act(() => store.set(refreshAtom, (v) => v + 1))
} finally {
console.error = originalConsoleError
}
await waitFor(() => assert(didThrow))
})

it('should not suspend the component', function test() {
Expand Down
38 changes: 11 additions & 27 deletions tests/syncEffect/syncEffect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ type StoreWithHooks = Store & {
type STORE_METHODS_TYPE = keyof {
[K in keyof Store as K extends symbol ? K : never]: K
}
type SecretStoreMethods = Store[STORE_METHODS_TYPE]

type EnsureAtomState = SecretStoreMethods[3]

type AtomState = ReturnType<EnsureAtomState>

type AnyAtom = Atom<unknown>

type GetterWithPeek = Getter & { peek: Getter }
Expand All @@ -41,14 +35,17 @@ type Ref = {
epoch: number
error?: unknown
isRefreshing?: boolean
deps: Map<AnyAtom, unknown>
}

export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
const refAtom = atom<Ref>(() => ({ epoch: 0 }))
const refreshAtom = atom(0)
const refAtom = atom<Ref>(() => ({ epoch: 0, deps: new Map() }))

const internalAtom = atom(function internalAtomRead(get) {
get(refreshAtom)
const ref = get(refAtom)
throwPendingError(ref)
Array.from(ref.deps.keys(), get)
return ++ref.epoch
})

Expand All @@ -73,12 +70,12 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
if (!isMounted || (inProgress && !ref.isRefreshing) || isRecursing) {
return
}
const deps = new Map<AnyAtom, unknown>()
ref.deps.clear()
let [get, set] = makeGetAndSet(true)

const getter: GetterWithPeek = ((a) => {
const value = get(a)
deps.set(a, value)
ref.deps.set(a, value)
return value
}) as GetterWithPeek

Expand Down Expand Up @@ -112,10 +109,6 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
hasChanged = false
runEffect()
}
// const depsChanged = Array.from(deps).some(areDifferent)
// if (depsChanged) {
// refresh()
// }
}
}

Expand Down Expand Up @@ -144,11 +137,7 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
}
}
}
} catch (error) {
ref.error = error
throw error
} finally {
Array.from(deps.keys(), get)
INTERNAL_mountDependencies(internalAtom, atomState)
INTERNAL_recomputeInvalidatedAtoms()
;[get, set] = makeGetAndSet(false)
Expand All @@ -159,13 +148,14 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
,
,
INTERNAL_ensureAtomState,
INTERNAL_readAtomState,
,
,
,
,
INTERNAL_mountDependencies,
INTERNAL_makeGetter,
INTERNAL_makeSetter,
,
INTERNAL_recomputeInvalidatedAtoms,
] = store[STORE_METHODS]

Expand All @@ -192,6 +182,7 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
syncEffectChannel.add(runEffect)
})
}
;(internalAtom as any).init = 0

if (process.env.NODE_ENV !== 'production') {
function setLabel(atom: Atom<unknown>, label: string) {
Expand All @@ -201,6 +192,7 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
atom.debugPrivate = true
}
setLabel(refAtom, 'ref')
setLabel(refreshAtom, 'refresh')
setLabel(internalAtom, 'internal')
}

Expand All @@ -215,14 +207,6 @@ export function syncEffect(effect: Effect): Atom<void> & { effect: Effect } {
return effectAtom
}

function throwPendingError(ref: Ref) {
if ('error' in ref) {
const error = ref.error
delete ref.error
throw error
}
}

function ensureSyncEffectChannel(store: Store) {
const storeWithHooks = store as StoreWithHooks
let syncEffectChannel = storeWithHooks[SYNC_EFFECT_CHANNEL]
Expand Down

0 comments on commit d751b83

Please sign in to comment.