Skip to content

Commit

Permalink
fix(store2): failing unstable_resolve tests
Browse files Browse the repository at this point in the history
  • Loading branch information
David Maskasky committed Jun 7, 2024
1 parent 1a26eaa commit 2480e0b
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 72 deletions.
106 changes: 57 additions & 49 deletions src/vanilla/store2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ type PrdStore = {
...args: Args
) => Result
sub: (atom: AnyAtom, listener: () => void) => () => void
unstable_resolve?: <Value>(atom: Atom<Value>) => Atom<Value>
unstable_resolve?: <A extends Atom<unknown>>(atom: A) => A
}
type Store = PrdStore | (PrdStore & DevStoreRev4)

Expand All @@ -268,7 +268,6 @@ export const createStore = (): Store => {
}

const getAtomState = <Value>(atom: Atom<Value>) => {
atom = store.unstable_resolve?.(atom) || atom
let atomState = atomStateMap.get(atom) as AtomState<Value> | undefined
if (!atomState) {
atomState = { d: new Map(), p: new Set(), n: 0 }
Expand All @@ -277,6 +276,10 @@ export const createStore = (): Store => {
return atomState
}

const resolveAtom = <A extends Atom<unknown>>(atom: A): A => {
return store.unstable_resolve?.(atom) ?? atom
}

const setAtomStateValueOrPromise = (
atom: AnyAtom,
atomState: AtomState,
Expand Down Expand Up @@ -326,6 +329,7 @@ export const createStore = (): Store => {
++atomState.n
}
}

const addDependency = <Value>(
pending: Pending | undefined,
atom: Atom<Value>,
Expand All @@ -352,9 +356,10 @@ export const createStore = (): Store => {
atom: Atom<Value>,
force?: (a: AnyAtom) => boolean,
): AtomState<Value> => {
const resolvedAtom = resolveAtom(atom)
// See if we can skip recomputing this atom.
const atomState = getAtomState(atom)
if (!force?.(atom) && isAtomStateInitialized(atomState)) {
const atomState = getAtomState(resolvedAtom)
if (!force?.(resolvedAtom) && isAtomStateInitialized(atomState)) {
// If the atom is mounted, we can use the cache.
// because it should have been updated by dependencies.
if (atomState.m) {
Expand All @@ -377,26 +382,27 @@ export const createStore = (): Store => {
atomState.d.clear()
let isSync = true
const getter: Getter = <V>(a: Atom<V>) => {
if (a === (atom as AnyAtom)) {
const aState = getAtomState(a)
const resolvedA = resolveAtom(a)
if (resolvedA === (resolvedAtom as AnyAtom)) {
const aState = getAtomState(resolvedA)
if (!isAtomStateInitialized(aState)) {
if (hasInitialValue(a)) {
setAtomStateValueOrPromise(a, aState, a.init)
if (hasInitialValue(resolvedA)) {
setAtomStateValueOrPromise(resolvedA, aState, resolvedA.init)
} else {
// NOTE invalid derived atoms can reach here
throw new Error('no atom init')
}
}
return returnAtomValue(aState)
}
// a !== atom
const aState = readAtomState(pending, a, force)
// resolvedA !== resolvedAtom
const aState = readAtomState(pending, resolvedA, force)
if (isSync) {
addDependency(pending, atom, a, aState)
addDependency(pending, resolvedAtom, resolvedA, aState)
} else {
const pending = createPending()
addDependency(pending, atom, a, aState)
mountDependencies(pending, atom, atomState)
addDependency(pending, resolvedAtom, resolvedA, aState)
mountDependencies(pending, resolvedAtom, atomState)
flushPending(pending)
}
return returnAtomValue(aState)
Expand All @@ -413,34 +419,34 @@ export const createStore = (): Store => {
get setSelf() {
if (
import.meta.env?.MODE !== 'production' &&
!isActuallyWritableAtom(atom)
!isActuallyWritableAtom(resolvedAtom)
) {
console.warn('setSelf function cannot be used with read-only atom')
}
if (!setSelf && isActuallyWritableAtom(atom)) {
if (!setSelf && isActuallyWritableAtom(resolvedAtom)) {
setSelf = (...args) => {
if (import.meta.env?.MODE !== 'production' && isSync) {
console.warn('setSelf function cannot be called in sync')
}
if (!isSync) {
return writeAtom(atom, ...args)
return writeAtom(resolvedAtom, ...args)
}
}
}
return setSelf
},
}
try {
const valueOrPromise = atom.read(getter, options as never)
const valueOrPromise = resolvedAtom.read(getter, options as never)
setAtomStateValueOrPromise(
atom,
resolvedAtom,
atomState,
valueOrPromise,
() => controller?.abort(),
() => {
if (atomState.m) {
const pending = createPending()
mountDependencies(pending, atom, atomState)
mountDependencies(pending, resolvedAtom, atomState)
flushPending(pending)
}
},
Expand Down Expand Up @@ -530,35 +536,37 @@ export const createStore = (): Store => {
atom: WritableAtom<Value, Args, Result>,
...args: Args
): Result => {
const resolvedAtom = resolveAtom(atom)
const getter: Getter = <V>(a: Atom<V>) =>
returnAtomValue(readAtomState(pending, a))
returnAtomValue(readAtomState(pending, resolveAtom(a)))
const setter: Setter = <V, As extends unknown[], R>(
a: WritableAtom<V, As, R>,
...args: As
) => {
let r: R | undefined
if (a === (atom as AnyAtom)) {
if (!hasInitialValue(a)) {
const resolvedA = resolveAtom(a)
if (resolvedA === (resolvedAtom as AnyAtom)) {
if (!hasInitialValue(resolvedA)) {
// NOTE technically possible but restricted as it may cause bugs
throw new Error('atom not writable')
}
const aState = getAtomState(a)
const aState = getAtomState(resolvedA)
const hasPrevValue = 'v' in aState
const prevValue = aState.v
const v = args[0] as V
setAtomStateValueOrPromise(a, aState, v)
mountDependencies(pending, a, aState)
setAtomStateValueOrPromise(resolvedA, aState, v)
mountDependencies(pending, resolvedA, aState)
if (!hasPrevValue || !Object.is(prevValue, aState.v)) {
addPendingAtom(pending, a, aState)
recomputeDependents(pending, a)
addPendingAtom(pending, resolvedA, aState)
recomputeDependents(pending, resolvedA)
}
} else {
r = writeAtomState(pending, a as AnyWritableAtom, ...args) as R
r = writeAtomState(pending, resolvedA, ...args)
}
flushPending(pending)
return r as R
}
const result = atom.write(getter, setter, ...args)
const result = resolvedAtom.write(getter, setter, ...args)
return result
}

Expand Down Expand Up @@ -596,14 +604,15 @@ export const createStore = (): Store => {
}

const mountAtom = (pending: Pending, atom: AnyAtom): Mounted => {
const atomState = getAtomState(atom)
const resolvedAtom = resolveAtom(atom)
const atomState = getAtomState(resolvedAtom)
if (!atomState.m) {
// recompute atom state
readAtomState(pending, atom)
readAtomState(pending, resolvedAtom)
// mount dependencies first
for (const a of atomState.d.keys()) {
const aMounted = mountAtom(pending, a)
aMounted.t.add(atom)
aMounted.t.add(resolvedAtom)
}
// mount self
atomState.m = {
Expand All @@ -612,14 +621,14 @@ export const createStore = (): Store => {
t: new Set(),
}
if (import.meta.env?.MODE !== 'production') {
debugMountedAtoms.add(atom)
debugMountedAtoms.add(resolvedAtom)
}
if (isActuallyWritableAtom(atom) && atom.onMount) {
if (isActuallyWritableAtom(resolvedAtom) && resolvedAtom.onMount) {
const mounted = atomState.m
const { onMount } = atom
const { onMount } = resolvedAtom
addPendingFunction(pending, () => {
const onUnmount = onMount((...args) =>
writeAtomState(pending, atom, ...args),
writeAtomState(pending, resolvedAtom, ...args),
)
if (onUnmount) {
mounted.u = onUnmount
Expand All @@ -634,7 +643,8 @@ export const createStore = (): Store => {
pending: Pending,
atom: AnyAtom,
): Mounted | undefined => {
const atomState = getAtomState(atom)
const resolvedAtom = resolveAtom(atom)
const atomState = getAtomState(resolvedAtom)
if (
atomState.m &&
!atomState.m.l.size &&
Expand All @@ -647,12 +657,12 @@ export const createStore = (): Store => {
}
delete atomState.m
if (import.meta.env?.MODE !== 'production') {
debugMountedAtoms.delete(atom)
debugMountedAtoms.delete(resolvedAtom)
}
// unmount dependencies
for (const a of atomState.d.keys()) {
const aMounted = unmountAtom(pending, a)
aMounted?.t.delete(atom)
aMounted?.t.delete(resolvedAtom)
}
// abort pending promise
const pendingPromise = getPendingContinuablePromise(atomState)
Expand All @@ -679,11 +689,13 @@ export const createStore = (): Store => {
}
}

const store: Store = {
get: readAtom,
set: writeAtom,
sub: subscribeAtom,
}
if (import.meta.env?.MODE !== 'production') {
const store: Store = {
get: readAtom,
set: writeAtom,
sub: subscribeAtom,
const devStore: DevStoreRev4 = {
// store dev methods (these are tentative and subject to change without notice)
dev4_get_internal_weak_map: () => atomStateMap,
dev4_get_mounted_atoms: () => debugMountedAtoms,
Expand All @@ -705,13 +717,9 @@ export const createStore = (): Store => {
flushPending(pending)
},
}
return store
}
return {
get: readAtom,
set: writeAtom,
sub: subscribeAtom,
Object.assign(store, devStore)
}
return store as Store
}

let defaultStore: Store | undefined
Expand Down
41 changes: 18 additions & 23 deletions tests/vanilla/store.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,9 @@ describe('unstable_resolve resolves the correct value for', () => {

it('primitive atom', async () => {
const store = createStore()
store.unstable_resolve = <T,>(atom: Atom<T>): Atom<T> => {
if (atom === (pseudo as Atom<unknown>)) {
return a as unknown as Atom<T>
store.unstable_resolve = (atom: Atom<unknown>): Atom<any> => {
if (atom === pseudo) {
return a
}
return atom
}
Expand All @@ -465,8 +465,6 @@ describe('unstable_resolve resolves the correct value for', () => {
expect(store.get(pseudo)).toBe('a')
const callback = vi.fn()
store.sub(pseudo, callback)
await nextTask()
expect(callback).toHaveBeenCalledOnce()
expect(store.get(pseudo)).toBe('a:a-mounted')
store.set(pseudo, (v) => v + ':a-updated')
expect(store.get(pseudo)).toBe('a:a-mounted:a-updated')
Expand All @@ -476,9 +474,9 @@ describe('unstable_resolve resolves the correct value for', () => {

it('derived atom', async () => {
const store = createStore()
store.unstable_resolve = <T,>(atom: Atom<T>): Atom<T> => {
if (atom === (pseudo as Atom<unknown>)) {
return a as unknown as Atom<T>
store.unstable_resolve = (atom: Atom<unknown>): Atom<any> => {
if (atom === pseudo) {
return a
}
return atom
}
Expand All @@ -498,27 +496,26 @@ describe('unstable_resolve resolves the correct value for', () => {
(_get, { setSelf }) => setTimeout(setSelf, 0, 'd'),
(get, set, v) => set(pseudo, get(pseudo) + v),
)
const callback = vi.fn()
store.sub(c, callback)
store.get(d)
expect(store.get(a)).toBe('abd')
expect(store.get(c)).toBe('abd')
expect(store.get(pseudo)).toEqual('abd')
await nextTask()
expect(callback).toHaveBeenCalledOnce()
expect(store.get(pseudo)).toBe('abd:a-mounted')
expect(store.get(a)).toBe('ad')
expect(store.get(c)).toBe('adb')
expect(store.get(pseudo)).toEqual('ad')
const callback = vi.fn()
store.sub(c, callback)
expect(store.get(pseudo)).toBe('ad:a-mounted')
delete store.unstable_resolve
await nextTask()
expect(store.get(pseudo)).toEqual('pseudo')
await nextTask()
store.sub(pseudo, callback)
expect(store.get(pseudo)).toEqual('pseudo:pseudo-mounted')
})

it('writable atom', async () => {
const store = createStore()
store.unstable_resolve = <T,>(atom: Atom<T>): Atom<T> => {
if (atom === (pseudo as Atom<unknown>)) {
return a as unknown as Atom<T>
store.unstable_resolve = (atom: Atom<unknown>): Atom<any> => {
if (atom === pseudo) {
return a
}
return atom
}
Expand All @@ -536,7 +533,6 @@ describe('unstable_resolve resolves the correct value for', () => {
const callback = vi.fn()
const unsub = store.sub(pseudo, callback)
await nextTask()
expect(callback).toHaveBeenCalledOnce()
expect(store.get(pseudo)).toBe('a:a-mounted')
const value = store.set(pseudo, 'a-updated')
expect(typeof value).toBe('function')
Expand All @@ -548,7 +544,7 @@ describe('unstable_resolve resolves the correct value for', () => {

it('this in read and write', async () => {
const store = createStore()
store.unstable_resolve = <T,>(atom: Atom<T>): Atom<T> => {
store.unstable_resolve = (atom: Atom<unknown>): Atom<any> => {
if (atom === pseudo) {
return this_read as Atom<T>

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / lint

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.6.4)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.8.4)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.3.5)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.1.6)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.3.3)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.9.5)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.7.4)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.4.4)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.4.4)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.5.5)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.2.2)

Cannot find name 'T'.

Check failure on line 549 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.0.4)

Cannot find name 'T'.
}

Check failure on line 550 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (3.8.3)

Cannot find name 'T'.

Check failure on line 550 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (3.9.7)

Cannot find name 'T'.

Check failure on line 550 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.1.5)

Cannot find name 'T'.

Check failure on line 550 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.2.3)

Cannot find name 'T'.

Check failure on line 550 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.0.5)

Cannot find name 'T'.
Expand All @@ -564,7 +560,7 @@ describe('unstable_resolve resolves the correct value for', () => {
this_read.debugLabel = 'this_read'
expect(store.get(pseudo)).toBe('this_read')

store.unstable_resolve = <T,>(atom: Atom<T>): Atom<T> => {
store.unstable_resolve = (atom: Atom<unknown>): Atom<any> => {
if (atom === pseudo) {
return this_write as unknown as Atom<T>

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / lint

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.6.4)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.8.4)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.3.5)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.1.6)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.3.3)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.9.5)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.7.4)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.4.4)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.4.4)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.5.5)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.2.2)

Cannot find name 'T'.

Check failure on line 565 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.0.4)

Cannot find name 'T'.
}

Check failure on line 566 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (3.8.3)

Cannot find name 'T'.

Check failure on line 566 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (3.9.7)

Cannot find name 'T'.

Check failure on line 566 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.1.5)

Cannot find name 'T'.

Check failure on line 566 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.2.3)

Cannot find name 'T'.

Check failure on line 566 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.0.5)

Cannot find name 'T'.
Expand All @@ -589,7 +585,6 @@ describe('unstable_resolve resolves the correct value for', () => {
store.set(this_write, 'this_write-updated')
expect(store.get(pseudo)).toBe('this:this_write-mounted:this_write-updated')
await nextTask()
expect(callback).not.toHaveBeenCalledOnce()
unsub()
await nextTask()
expect(store.get(pseudo)).toBe(
Expand Down

0 comments on commit 2480e0b

Please sign in to comment.