Skip to content

Commit 10bb508

Browse files
committed
refactor(connectors): resolve selected option labels with useQueries
Replaces the remembered-label map with per-id queries over the selected values, following the pattern the knowledge-base selector already uses. The map only ever held ids searched for in the current session, so a multi-select field restored from saved config still rendered raw ids — the case that actually matters. It also needed two effects and a serialized-context key to avoid leaking labels across a context change, all of which disappear: queries key on the context, so a label cannot outlive it. Gates the speculative lookup of typed text on a new `resolvesUnknownIds` flag, set only where `fetchById` returns null for an id that does not exist. Most implementations resolve a record by id, so every partial keystroke was a failed upstream request, retried once, and its error made "could not check that exact value" the normal empty state on those selectors. Also: pass handlers straight to the combobox rather than through identity wrappers that defeated its memo, move the latest-callback ref write into an effect, rename the raw search setter so the deduping write path cannot be bypassed, and correct the prop and staleTime docs.
1 parent e922623 commit 10bb508

6 files changed

Lines changed: 118 additions & 83 deletions

File tree

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx

Lines changed: 39 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useEffect, useMemo, useState } from 'react'
3+
import { useMemo, useState } from 'react'
44
import { ChipCombobox, type ComboboxOption, Loader } from '@sim/emcn'
55
import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state'
66
import { SELECTOR_CONTEXT_FIELDS } from '@/lib/workflows/subblocks/context'
@@ -10,8 +10,13 @@ import type {
1010
} from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields'
1111
import { getDependsOnFields } from '@/blocks/utils'
1212
import type { ConnectorConfigField } from '@/connectors/types'
13-
import type { SelectorContext, SelectorKey, SelectorOption } from '@/hooks/selectors/types'
14-
import { useSelectorOptionDetail, useSelectorOptions } from '@/hooks/selectors/use-selector-query'
13+
import { getSelectorDefinition } from '@/hooks/selectors/registry'
14+
import type { SelectorContext, SelectorKey } from '@/hooks/selectors/types'
15+
import {
16+
useSelectorOptionDetail,
17+
useSelectorOptionDetails,
18+
useSelectorOptions,
19+
} from '@/hooks/selectors/use-selector-query'
1520
import { useDebounce } from '@/hooks/use-debounce'
1621

1722
interface ConnectorSelectorFieldProps {
@@ -78,83 +83,55 @@ export function ConnectorSelectorField({
7883
})
7984

8085
/**
81-
* The option list fills by draining pages in the background and the combobox
82-
* filters it client-side, so an option is only findable once its page has
83-
* arrived. Resolving the typed value directly makes an exact id/key selectable
84-
* immediately, independent of drain progress. Debounced so typing does not
85-
* issue a request per keystroke; selectors without a `fetchById` resolve nothing.
86+
* Label every selected value, including values restored from saved config that no
87+
* in-session search would have resolved. Queries are keyed on `context`, so a label
88+
* can never outlive the context that produced it, and they share keys with the
89+
* speculative lookup below so an already-resolved id costs no extra request.
8690
*/
87-
const debouncedSearch = useDebounce(searchTerm.trim(), SEARCH_DEBOUNCE_MS)
88-
const { data: searchedOption, error: searchError } = useSelectorOptionDetail(field.selectorKey, {
91+
const singleValue = Array.isArray(value) ? value[0] : value
92+
const selectedIds = useMemo(
93+
() => (Array.isArray(value) ? value : value ? [value] : []).filter(Boolean),
94+
[value]
95+
)
96+
const selectedOptions = useSelectorOptionDetails(field.selectorKey, {
8997
context,
90-
detailId: isEnabled && debouncedSearch.length > 0 ? debouncedSearch : undefined,
98+
detailIds: isEnabled ? selectedIds : [],
9199
})
92100

93101
/**
94-
* Resolve the *selected* value too, not just the typed one, so the trigger does
95-
* not fall back to a raw id for something just picked. Single-select only:
96-
* resolving N ids would need N hooks, so multi-select relies on the remembered
97-
* options below.
102+
* The option list fills by draining pages in the background and the combobox filters
103+
* it client-side, so an option is only findable once its page has arrived. Where the
104+
* selector's `fetchById` tolerates an unknown id, whatever the user typed is resolved
105+
* directly so an exact key is selectable immediately. Gated on that flag because most
106+
* implementations resolve a record by id, where a partial keystroke is a guaranteed
107+
* failed upstream request rather than an empty result.
98108
*/
99-
const singleValue = Array.isArray(value) ? value[0] : value
100-
const { data: selectedOption } = useSelectorOptionDetail(field.selectorKey, {
109+
const resolvesUnknownIds = Boolean(getSelectorDefinition(field.selectorKey).resolvesUnknownIds)
110+
const debouncedSearch = useDebounce(searchTerm.trim(), SEARCH_DEBOUNCE_MS)
111+
const { data: searchedOption } = useSelectorOptionDetail(field.selectorKey, {
101112
context,
102-
detailId: !isMulti && isEnabled && singleValue ? singleValue : undefined,
113+
detailId:
114+
resolvesUnknownIds && isEnabled && debouncedSearch.length > 0 ? debouncedSearch : undefined,
103115
})
104116

105117
const emptyMessage = getEmptyMessage(field.title.toLowerCase(), {
106118
error,
107-
lookupFailed: Boolean(searchError),
108119
hasMore,
109120
isFetchingMore,
110121
truncated,
111122
})
112123

113-
/**
114-
* Resolved options are remembered for the lifetime of the field. Both lookups are
115-
* keyed on values that change — the search box clears on select and close, and a
116-
* multi-select field resolves no id at all (that would need one hook per id) — so
117-
* reading them directly would drop a label moments after it appeared, leaving the
118-
* trigger showing a raw id for something the user just picked.
119-
*/
120-
const [resolvedOptions, setResolvedOptions] = useState<Record<string, string>>({})
121-
/**
122-
* Ids are only meaningful within one selector context, so switching credential,
123-
* domain, or a dependency must drop what was resolved under the old one — the
124-
* queries re-key, but remembered labels would otherwise linger and mislabel.
125-
* Keyed on the serialized context rather than its identity: the memo also depends
126-
* on `sourceConfig`, so its identity changes on unrelated field edits.
127-
*/
128-
const contextKey = useMemo(() => JSON.stringify(context), [context])
129-
useEffect(() => {
130-
setResolvedOptions((prev) => (Object.keys(prev).length > 0 ? {} : prev))
131-
}, [contextKey])
132-
133-
useEffect(() => {
134-
const found = [searchedOption, selectedOption].filter(Boolean) as SelectorOption[]
135-
if (found.length === 0) return
136-
setResolvedOptions((prev) => {
137-
let next = prev
138-
for (const option of found) {
139-
if (next[option.id] === option.label) continue
140-
if (next === prev) next = { ...prev }
141-
next[option.id] = option.label
142-
}
143-
return next
144-
})
145-
}, [searchedOption, selectedOption])
146-
147124
const comboboxOptions = useMemo<ComboboxOption[]>(() => {
148125
const base = options.map((opt) => ({ label: opt.label, value: opt.id }))
149126
const seen = new Set(base.map((opt) => opt.value))
150127
const extras: ComboboxOption[] = []
151-
for (const [id, label] of Object.entries(resolvedOptions)) {
152-
if (seen.has(id)) continue
153-
seen.add(id)
154-
extras.push({ label, value: id })
128+
for (const option of searchedOption ? [...selectedOptions, searchedOption] : selectedOptions) {
129+
if (seen.has(option.id)) continue
130+
seen.add(option.id)
131+
extras.push({ label: option.label, value: option.id })
155132
}
156133
return extras.length > 0 ? [...extras, ...base] : base
157-
}, [options, resolvedOptions])
134+
}, [options, selectedOptions, searchedOption])
158135

159136
if (isLoading && isEnabled) {
160137
return (
@@ -172,7 +149,7 @@ export function ConnectorSelectorField({
172149
multiSelect
173150
options={comboboxOptions}
174151
multiSelectValues={multiValues}
175-
onMultiSelectChange={(values) => onChange(values)}
152+
onMultiSelectChange={onChange}
176153
searchable
177154
onSearchChange={setSearchTerm}
178155
searchPlaceholder={`Search ${field.title.toLowerCase()}...`}
@@ -193,7 +170,7 @@ export function ConnectorSelectorField({
193170
<ChipCombobox
194171
options={comboboxOptions}
195172
value={singleValue || undefined}
196-
onChange={(next) => onChange(next)}
173+
onChange={onChange}
197174
searchable
198175
onSearchChange={setSearchTerm}
199176
searchPlaceholder={`Search ${field.title.toLowerCase()}...`}
@@ -222,19 +199,16 @@ function getEmptyMessage(
222199
noun: string,
223200
state: {
224201
error: Error | null
225-
lookupFailed: boolean
226202
hasMore: boolean
227203
isFetchingMore: boolean
228204
truncated: boolean
229205
}
230206
): string {
231-
// `field.title` is singular on some connectors ("Base") and plural on others
232-
// ("Spaces"), so only the settled message puts the noun behind a quantifier.
233207
if (state.error) return 'No match — the list failed to load. Try reopening'
234-
// Distinct from the list failing: the list is fine, resolving the typed value is not.
235-
if (state.lookupFailed) return 'No match — could not check that exact value'
236208
if (state.hasMore || state.isFetchingMore) return 'No match yet — still loading…'
237209
if (state.truncated) return 'No match — too many to list. Try a more exact term'
210+
// `noun` is singular on some connectors ("Base") and plural on others ("Spaces"),
211+
// so only this settled message puts it behind a quantifier.
238212
return `No ${noun} found`
239213
}
240214

apps/sim/hooks/selectors/providers/confluence/selectors.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@ export const confluenceSelectors = {
4949
nextCursor: data.nextCursor,
5050
}
5151
},
52+
/** The server filters by key and returns nothing for a key that does not exist. */
53+
resolvesUnknownIds: true,
5254
/**
53-
* Resolves a single space by key in one request via the server's exact-key
54-
* lookup. Previously this fetched the first page and scanned it, so a space
55-
* that sorts beyond page 1 never resolved — on a large site that is most of
56-
* them. Keyed resolution is independent of how far the page drain has run.
55+
* Resolves a single space by key via the server's exact-key lookup, independent
56+
* of how far the page drain has run — a space sorting beyond page 1 would
57+
* otherwise never resolve, which on a large site is most of them.
5758
*/
5859
fetchById: async ({ context, detailId, signal }: SelectorQueryArgs) => {
5960
if (!detailId) return null

apps/sim/hooks/selectors/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ export interface SelectorDefinition {
136136
*/
137137
fetchPage?: (args: SelectorPageArgs) => Promise<SelectorPage>
138138
fetchById?: (args: SelectorQueryArgs) => Promise<SelectorOption | null>
139+
/**
140+
* Set when `fetchById` tolerates an id that may not exist, returning `null` rather
141+
* than erroring. Only then is it safe to speculatively resolve whatever a user has
142+
* typed — most implementations resolve a record by id and would turn every partial
143+
* keystroke into a failed upstream request.
144+
*/
145+
resolvesUnknownIds?: boolean
139146
enabled?: (args: SelectorQueryArgs) => boolean
140147
staleTime?: number
141148
}

apps/sim/hooks/selectors/use-selector-query.ts

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useEffect, useMemo } from 'react'
22
import { createLogger } from '@sim/logger'
3-
import { useInfiniteQuery, useQuery } from '@tanstack/react-query'
3+
import { useInfiniteQuery, useQueries, useQuery } from '@tanstack/react-query'
44
import { extractEnvVarName, isEnvVarReference, isReference } from '@/executor/constants'
55
import { usePersonalEnvironment } from '@/hooks/queries/environment'
66
import { getSelectorDefinition, mergeOption } from '@/hooks/selectors/registry'
@@ -55,8 +55,8 @@ const MAX_AUTO_DRAIN_PAGES = 50
5555
export const DEFAULT_SELECTOR_STALE_TIME = 30_000
5656

5757
/**
58-
* Single-option resolutions are keyed by an exact id, so they change far less
59-
* often than a list and can stay fresh longer.
58+
* Fallback for a single-option resolution when the definition declares no
59+
* `staleTime`: keyed by an exact id, so it changes far less often than a list.
6060
*/
6161
export const DEFAULT_SELECTOR_DETAIL_STALE_TIME = 300_000
6262

@@ -192,9 +192,8 @@ export function useSelectorOptionDetail(
192192
* Callers that pass nothing keep the list predicate as their default.
193193
*/
194194
const enabled =
195-
args.enabled !== undefined
196-
? args.enabled && canResolveDetail
197-
: canResolveDetail && (definition.enabled ? definition.enabled(queryArgs) : true)
195+
(args.enabled ?? (definition.enabled ? definition.enabled(queryArgs) : true)) &&
196+
canResolveDetail
198197

199198
const query = useQuery<SelectorOption | null>({
200199
queryKey: [...definition.getQueryKey(queryArgs), 'detail', resolvedDetailId ?? 'none'],
@@ -206,6 +205,53 @@ export function useSelectorOptionDetail(
206205
return query
207206
}
208207

208+
/**
209+
* Resolves several ids at once, so a multi-select field can label every selected
210+
* value — including values restored from saved config, which no in-session search
211+
* would have resolved. Query keys match {@link useSelectorOptionDetail} exactly, so
212+
* the two share a cache and an id already resolved by search costs no extra request.
213+
*/
214+
export function useSelectorOptionDetails(
215+
key: SelectorKey,
216+
args: Omit<SelectorHookArgs, 'detailId'> & { detailIds: string[] }
217+
): SelectorOption[] {
218+
const { data: envVariables = {} } = usePersonalEnvironment()
219+
const definition = getSelectorDefinition(key)
220+
221+
const resolvedIds = useMemo(() => {
222+
const out: string[] = []
223+
for (const id of args.detailIds) {
224+
if (!id || isReference(id)) continue
225+
if (isEnvVarReference(id)) {
226+
const value = envVariables[extractEnvVarName(id)]?.value
227+
if (value) out.push(value)
228+
continue
229+
}
230+
out.push(id)
231+
}
232+
return Array.from(new Set(out))
233+
}, [args.detailIds, envVariables])
234+
235+
const results = useQueries({
236+
queries: resolvedIds.map((detailId) => {
237+
const queryArgs: SelectorQueryArgs = { key, context: args.context, detailId }
238+
const canResolveDetail = definition.fetchById !== undefined
239+
return {
240+
queryKey: [...definition.getQueryKey(queryArgs), 'detail', detailId],
241+
queryFn: ({ signal }: { signal: AbortSignal }) =>
242+
definition.fetchById!({ ...queryArgs, signal }),
243+
enabled:
244+
args.enabled !== undefined
245+
? args.enabled && canResolveDetail
246+
: canResolveDetail && (definition.enabled ? definition.enabled(queryArgs) : true),
247+
staleTime: definition.staleTime ?? DEFAULT_SELECTOR_DETAIL_STALE_TIME,
248+
}
249+
}),
250+
})
251+
252+
return useMemo(() => results.flatMap((result) => (result.data ? [result.data] : [])), [results])
253+
}
254+
209255
export function useSelectorOptionMap(options: SelectorOption[], extra?: SelectorOption | null) {
210256
return useMemo(() => {
211257
const merged = mergeOption(options, extra)

apps/sim/lib/api/contracts/selectors/confluence.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,11 @@ export const confluenceSpacesSelectorBodySchema = credentialWorkflowDomainBodySc
371371
* `/spaces` supports a `keys` filter, so a known key resolves in one request
372372
* instead of depending on how far the background page drain has progressed.
373373
*/
374-
spaceKey: z.string().min(1).max(255).optional(),
374+
spaceKey: z
375+
.string()
376+
.min(1, 'spaceKey cannot be empty')
377+
.max(255, 'spaceKey must be 255 characters or fewer')
378+
.optional(),
375379
})
376380

377381
export const confluenceSpacesSelectorContract = definePostSelector(

packages/emcn/src/components/combobox/combobox.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,10 @@ export interface ComboboxProps
119119
/** Enable search input in dropdown (useful for multiselect) */
120120
searchable?: boolean
121121
/**
122-
* Notified when the dropdown's search box changes, and with `''` whenever the
123-
* query is reset (select, close, blur, Escape, ArrowLeft) — including when
124-
* `searchable` is false, since those resets are unconditional.
122+
* Notified when the dropdown's search box changes value, including the `''` a
123+
* select, close, Escape, or ArrowLeft resets it to. Deduped, so an already-empty
124+
* query resetting again is silent and a consumer sees nothing while `searchable`
125+
* is false.
125126
*
126127
* This is the `searchable` search box only. In `editable` mode the typed text
127128
* arrives via `onChange`, not here.
@@ -197,25 +198,27 @@ const Combobox = memo(
197198
const listboxId = useId()
198199
const [open, setOpen] = useState(false)
199200
const [highlightedIndex, setHighlightedIndex] = useState(-1)
200-
const [searchQuery, setSearchQuery] = useState('')
201+
const [searchQuery, setSearchQueryState] = useState('')
201202
/**
202203
* Read through a ref so `updateSearchQuery` keeps a stable identity —
203204
* `handleSelect`, `handleBlur`, and `handleKeyDown` all capture it without
204205
* listing it as a dependency.
205206
*/
206207
const onSearchChangeRef = useRef(onSearchChange)
207-
onSearchChangeRef.current = onSearchChange
208+
useEffect(() => {
209+
onSearchChangeRef.current = onSearchChange
210+
}, [onSearchChange])
208211
/**
209212
* Single write path for the search box so `onSearchChange` cannot be missed on a
210213
* reset. Deduped because several paths reset redundantly — Escape both handles the
211214
* key and lets the popover dismiss, and an editable select blurs after selecting —
212-
* which `setSearchQuery` absorbed silently but a consumer callback would not.
215+
* which the raw setState absorbed silently but a consumer callback would not.
213216
*/
214217
const searchQueryRef = useRef('')
215218
const updateSearchQuery = useCallback((next: string) => {
216219
if (searchQueryRef.current === next) return
217220
searchQueryRef.current = next
218-
setSearchQuery(next)
221+
setSearchQueryState(next)
219222
onSearchChangeRef.current?.(next)
220223
}, [])
221224
const searchInputRef = useRef<HTMLInputElement>(null)

0 commit comments

Comments
 (0)