Skip to content

Commit 1cc2ce6

Browse files
authored
Fix tiny UI race after add SSH key in instance create form (#3177)
It's barely worth fixing, but codex flagged it in #3173 and it's real. Watch the `select all` checkbox below when I submit the create SSH key form. In the very short interval of time after the key is created and but before list of keys refetches, the select all checkbox flashes checked (all are selected) before flipping back to intermediate when the list comes in. It's trivial and it's not worth keeping the e2e test I used to prove and fix it (removed in 966aac1), but it is worth fixing now that I know about. https://github.com/user-attachments/assets/0bc9e5ff-ceb0-49b5-856d-8998a8971528 The checkbox showing the wrong thing is not actually the entire bug — if the user toggles select all during this interval, the behavior is slightly weird because the list of keys is out of sync, but it's nearly impossible for a human user to move fast enough to run into the issue, and the state after the request comes back will be up to date. The real fix would be to make `onSuccess` wait until after the invalidate is complete. But we don't have any other examples of `async onSuccess`, so I don't want to rush that fix in. ```diff const handleDismiss = onDismiss ? onDismiss : () => navigate(pb.sshKeys()) const createSshKey = useApiMutation(api.currentUserSshKeyCreate, { - onSuccess(sshKey) { - queryClient.invalidateEndpoint('currentUserSshKeyList') + async onSuccess(sshKey) { + await queryClient.invalidateEndpoint('currentUserSshKeyList') onSuccess?.(sshKey) handleDismiss() // prettier-ignore ```
1 parent 0df8ff0 commit 1cc2ce6

File tree

3 files changed

+38
-2
lines changed

3 files changed

+38
-2
lines changed

app/components/form/fields/SshKeysField.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { EmptyMessage } from '~/ui/lib/EmptyMessage'
2020
import { FieldLabel } from '~/ui/lib/FieldLabel'
2121
import { Message } from '~/ui/lib/Message'
2222
import { TextInputHint } from '~/ui/lib/TextInput'
23+
import { isSubset } from '~/util/array'
2324

2425
import { CheckboxField } from './CheckboxField'
2526
import { ErrorMessage } from './ErrorMessage'
@@ -73,7 +74,16 @@ export function SshKeysField({
7374
},
7475
})
7576

76-
const allAreSelected = allKeys.length === selectedKeys.length
77+
// do this with a subset instead of just counting the items because there's a
78+
// weird window right after adding but before the invalidate has gone through
79+
// where you can have the new one selected but it's not in the list of all
80+
// keys, which can cause the counts to match even though the sets don't
81+
const allAreSelected =
82+
allKeys.length > 0 &&
83+
isSubset(
84+
allKeys.map((k) => k.id),
85+
new Set(selectedKeys)
86+
)
7787

7888
return (
7989
<div className="max-w-lg">

app/util/array.spec.tsx

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,14 @@
88
import type { JSX, ReactElement } from 'react'
99
import { expect, test } from 'vitest'
1010

11-
import { groupBy, intersperse, isSetEqual, setDiff, setIntersection } from './array'
11+
import {
12+
groupBy,
13+
intersperse,
14+
isSetEqual,
15+
isSubset,
16+
setDiff,
17+
setIntersection,
18+
} from './array'
1219

1320
test('groupBy', () => {
1421
expect(
@@ -73,6 +80,17 @@ test('isSetEqual', () => {
7380
expect(isSetEqual(new Set([{}]), new Set([{}]))).toBe(false)
7481
})
7582

83+
test('isSubset', () => {
84+
expect(isSubset(new Set(), new Set())).toBe(true)
85+
expect(isSubset(new Set(), new Set(['a']))).toBe(true)
86+
expect(isSubset(new Set(['a']), new Set(['a', 'b']))).toBe(true)
87+
expect(isSubset(new Set(['a', 'b']), new Set(['a', 'b']))).toBe(true)
88+
89+
expect(isSubset(new Set(['a']), new Set())).toBe(false)
90+
expect(isSubset(new Set(['a', 'b']), new Set(['a']))).toBe(false)
91+
expect(isSubset(new Set(['c']), new Set(['a', 'b']))).toBe(false)
92+
})
93+
7694
test('setDiff', () => {
7795
expect(setDiff(new Set(), new Set())).toEqual(new Set())
7896
expect(setDiff(new Set(['a']), new Set())).toEqual(new Set(['a']))

app/util/array.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ export function isSetEqual<T>(a: Set<T>, b: Set<T>): boolean {
4949
return true
5050
}
5151

52+
/** Is `a ⊆ b`? */
53+
export function isSubset<T>(a: Iterable<T>, b: ReadonlySet<T>): boolean {
54+
for (const item of a) {
55+
if (!b.has(item)) return false
56+
}
57+
return true
58+
}
59+
5260
/** Set `a - b` */
5361
export function setDiff<T>(a: ReadonlySet<T>, b: ReadonlySet<T>): Set<T> {
5462
return new Set([...a].filter((x) => !b.has(x)))

0 commit comments

Comments
 (0)