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

fixed issue with .has() when new value is added and then deleted or c… #981

Merged
merged 9 commits into from
Oct 27, 2024
16 changes: 10 additions & 6 deletions src/vanilla/utils/proxyMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
type InternalProxyObject<K, V> = Map<K, V> & {
data: Array<V>
index: number
epoch: number
toJSON: () => Map<K, V>
}

Expand Down Expand Up @@ -68,6 +69,7 @@
const vObject: InternalProxyObject<K, V> = {
data: initialData,
index: initialIndex,
epoch: 0,
get size() {
if (!isProxy(this)) {
registerSnapMap()
Expand All @@ -80,19 +82,17 @@
const index = map.get(key)
if (index === undefined) {
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.index // touch property for tracking
this.epoch // touch property for tracking
return undefined
}
return this.data[index]
},
has(key: K) {
const map = getMapForThis(this)
const exists = map.has(key)

Check warning on line 92 in src/vanilla/utils/proxyMap.ts

View workflow job for this annotation

GitHub Actions / lint

'exists' is assigned a value but never used. Allowed unused vars must match /^_/u
if (!exists) {
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.index // touch property for tracking
}
return exists
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.epoch // touch property for tracking
return map.has(key)
},
set(key: K, value: V) {
if (!isProxy(this)) {
Expand All @@ -105,6 +105,7 @@
} else {
this.data[index] = value
}
this.epoch++
return this
},
delete(key: K) {
Expand All @@ -117,6 +118,7 @@
}
delete this.data[index]
indexMap.delete(key)
this.epoch++
return true
},
clear() {
Expand All @@ -125,6 +127,7 @@
}
this.data.length = 0 // empty array
this.index = 0
this.epoch++
indexMap.clear()
},
forEach(cb: (value: V, key: K, map: Map<K, V>) => void) {
Expand Down Expand Up @@ -166,6 +169,7 @@
Object.defineProperties(proxiedObject, {
size: { enumerable: false },
index: { enumerable: false },
epoch: { enumerable: false },
data: { enumerable: false },
toJSON: { enumerable: false },
})
Expand Down
16 changes: 10 additions & 6 deletions src/vanilla/utils/proxySet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ type InternalProxySet<T> = Set<T> & {
data: T[]
toJSON: object
index: number
epoch: number
intersection: (other: Set<T>) => Set<T>
isDisjointFrom: (other: Set<T>) => boolean
isSubsetOf: (other: Set<T>) => boolean
Expand Down Expand Up @@ -63,6 +64,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
const vObject: InternalProxySet<T> = {
data: initialData,
index: initialIndex,
epoch: 0,
get size() {
if (!isProxy(this)) {
registerSnapMap()
Expand All @@ -72,12 +74,9 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
has(value: T) {
const map = getMapForThis(this)
const v = maybeProxify(value)
const exists = map.has(v)
if (!exists) {
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.index // touch property for tracking
}
return exists
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
this.epoch // touch property for tracking
return map.has(v)
},
add(value: T) {
if (!isProxy(this)) {
Expand All @@ -87,6 +86,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
if (!indexMap.has(v)) {
indexMap.set(v, this.index)
this.data[this.index++] = v
this.epoch++
}
return this
},
Expand All @@ -101,6 +101,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
}
delete this.data[index]
indexMap.delete(v)
this.epoch++
return true
},
clear() {
Expand All @@ -109,6 +110,7 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
}
this.data.length = 0 // empty array
this.index = 0
this.epoch++
indexMap.clear()
},
forEach(cb) {
Expand Down Expand Up @@ -200,6 +202,8 @@ export function proxySet<T>(initialValues?: Iterable<T> | null) {
Object.defineProperties(proxiedObject, {
size: { enumerable: false },
data: { enumerable: false },
index: { enumerable: false },
epoch: { enumerable: false },
toJSON: { enumerable: false },
})
Object.seal(proxiedObject)
Expand Down
247 changes: 247 additions & 0 deletions tests/proxyMap.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ describe('proxyMap internal', () => {
).toBe(false)
})
})

describe('snapshot', () => {
it('should error when trying to mutate a snapshot', () => {
const state = proxyMap()
Expand Down Expand Up @@ -375,3 +376,249 @@ describe('snapshot', () => {
expect(snap2.get('key1')).toBe('val1modified')
})
})

describe('ui updates - useSnapshot', async () => {
it('should update ui when calling has before and after setting and deleting a key', async () => {
const state = proxyMap()
const TestComponent = () => {
const snap = useSnapshot(state)

return (
<>
<p>has key: {`${snap.has('key')}`}</p>
<button onClick={() => state.set('key', 'value')}>set key</button>
overthemike marked this conversation as resolved.
Show resolved Hide resolved
<button onClick={() => state.delete('key')}>delete key</button>
</>
)
}

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

await waitFor(() => {
getByText('has key: false')
})

fireEvent.click(getByText('set key'))
await waitFor(() => {
getByText('has key: true')
})

fireEvent.click(getByText('delete key'))
await waitFor(() => {
getByText('has key: false')
})
})

it('should update ui when calling has before and after settiing and deleting multiple keys', async () => {
const state = proxyMap()
const TestComponent = () => {
const snap = useSnapshot(state)

return (
<>
<p>has key: {`${snap.has('key')}`}</p>
<p>has key2: {`${snap.has('key2')}`}</p>
overthemike marked this conversation as resolved.
Show resolved Hide resolved
<button
onClick={() => {
state.set('key', 'value')
state.set('key2', 'value')
}}
>
set keys
</button>
<button
onClick={() => {
state.delete('key')
state.delete('key2')
}}
>
delete keys
</button>
</>
)
}

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

await waitFor(() => {
getByText('has key: false')
getByText('has key2: false')
})

fireEvent.click(getByText('set keys'))
await waitFor(() => {
getByText('has key: true')
getByText('has key2: true')
})

fireEvent.click(getByText('delete keys'))
await waitFor(() => {
getByText('has key: false')
getByText('has key2: false')
})
})

it('should update ui when calling has before and after settiing multile keys and deleting a single one (first item)', async () => {
const state = proxyMap()
const TestComponent = () => {
const snap = useSnapshot(state)

return (
<>
<p>has key: {`${snap.has('key')}`}</p>
<p>has key2: {`${snap.has('key2')}`}</p>
<button
onClick={() => {
state.set('key', 'value')
state.set('key2', 'value')
}}
>
set keys
</button>
<button
onClick={() => {
state.delete('key')
}}
>
delete keys
</button>
</>
)
}

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

await waitFor(() => {
getByText('has key: false')
getByText('has key2: false')
})

fireEvent.click(getByText('set keys'))
await waitFor(() => {
getByText('has key: true')
getByText('has key2: true')
})

fireEvent.click(getByText('delete keys'))
await waitFor(() => {
getByText('has key: false')
getByText('has key2: true')
})
})

it('should update ui when calling has before and after settiing multile keys and deleting a single one (first item)', async () => {
const state = proxyMap()
const TestComponent = () => {
const snap = useSnapshot(state)

return (
<>
<p>has key: {`${snap.has('key')}`}</p>
<p>has key2: {`${snap.has('key2')}`}</p>
<button
onClick={() => {
state.set('key', 'value')
state.set('key2', 'value')
}}
>
set keys
</button>
<button
onClick={() => {
state.delete('key2')
}}
>
delete keys
</button>
</>
)
}

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

await waitFor(() => {
getByText('has key: false')
getByText('has key2: false')
})

fireEvent.click(getByText('set keys'))
await waitFor(() => {
getByText('has key: true')
getByText('has key2: true')
})

fireEvent.click(getByText('delete keys'))
await waitFor(() => {
getByText('has key: true')
getByText('has key2: false')
})
})

it('should update ui when clearing the map', async () => {
const state = proxyMap()
const TestComponent = () => {
const snap = useSnapshot(state)

return (
<>
<p>has key: {`${snap.has('key')}`}</p>
<p>has key2: {`${snap.has('key2')}`}</p>
<button
onClick={() => {
state.set('key', 'value')
state.set('key2', 'value')
}}
>
set keys
</button>
<button
onClick={() => {
state.clear()
}}
>
clear map
</button>
</>
)
}

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

await waitFor(() => {
getByText('has key: false')
getByText('has key2: false')
})

fireEvent.click(getByText('set keys'))
await waitFor(() => {
getByText('has key: true')
getByText('has key2: true')
})

fireEvent.click(getByText('clear map'))
await waitFor(() => {
getByText('has key: false')
getByText('has key2: false')
})
})
})
Loading