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
9 changes: 3 additions & 6 deletions src/vanilla/utils/proxyMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,9 @@ export function proxyMap<K, V>(entries?: Iterable<[K, V]> | undefined | null) {
},
has(key: K) {
const map = getMapForThis(this)
const exists = map.has(key)
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.data.length // touch property for tracking
overthemike marked this conversation as resolved.
Show resolved Hide resolved
return map.has(key)
},
set(key: K, value: V) {
if (!isProxy(this)) {
Expand Down
9 changes: 3 additions & 6 deletions src/vanilla/utils/proxySet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,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.data.length // touch property for tracking
return map.has(v)
},
add(value: T) {
if (!isProxy(this)) {
Expand Down
38 changes: 38 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,40 @@ describe('snapshot', () => {
expect(snap2.get('key1')).toBe('val1modified')
})
})

describe('ui updates - useSnapshot', async () => {
it('should update ui when calling has before and after 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')
})
})
})
37 changes: 37 additions & 0 deletions tests/proxySet.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -365,3 +365,40 @@ describe('snapshot behavior', () => {
expect(snap2.has('val2')).toBe(true)
})
})

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

return (
<>
<p>has value: {`${snap.has('value')}`}</p>
<button onClick={() => state.add('value')}>add value</button>
<button onClick={() => state.delete('value')}>delete value</button>
</>
)
}

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

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

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

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