Skip to content

Commit cc8e7e2

Browse files
icecrasher321claude
andcommitted
fix(queries): scope the sub-block label cache by the selector's own context
Follow-on to 1f423ab, and a real gap in it. That commit taught `fetchById` to read sibling context but left the React Query key at `(workspaceId, blockId, subBlockId, optionId)`. A label resolved before its sibling was set — `workspace.credentialGroupProviders` with no group picked, which returns `null` — stayed cached under the same key and was reused once the group WAS picked, so the card kept showing the raw id. Changing between two groups collided the same way. This is the repo's own React Query rule ("every identifier the queryFn forwards into the fetch must appear in the queryKey"); `check:react-query` did not catch it because the context is built in the hook rather than passed as a named arg. The key now carries the selector's OWN `getQueryKey` for that context, rather than a second hand-maintained list of context fields. The cache is scoped by exactly what the selector reads, and stays correct if a selector's dependencies change later. The context also became reactive (subscribed rather than read via `getState()`), which is what lets the key move when the sibling does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1f423ab commit cc8e7e2

2 files changed

Lines changed: 68 additions & 14 deletions

File tree

apps/sim/hooks/queries/dynamic-subblock-options.test.tsx

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ vi.mock('@/hooks/selectors/registry', () => ({
1515
}))
1616

1717
import type { SubBlockConfig } from '@/blocks/types'
18-
import { useDynamicSubBlockOptionDisplayName } from '@/hooks/queries/dynamic-subblock-options'
18+
import {
19+
dynamicSubBlockOptionKeys,
20+
useDynamicSubBlockOptionDisplayName,
21+
} from '@/hooks/queries/dynamic-subblock-options'
1922
import type { SelectorDefinition, SelectorKey } from '@/hooks/selectors/types'
2023

2124
/** Any registered key; the hook only uses it to look the definition up. */
@@ -126,4 +129,22 @@ describe('useDynamicSubBlockOptionDisplayName', () => {
126129

127130
await waitForResult(() => expect(hook.result()).toBe('Gmail, Slack'))
128131
})
132+
133+
it('re-resolves a label when the sibling its selector depends on changes', () => {
134+
// The bug: `fetchById` reads sibling context, but the cache key did not, so a label
135+
// resolved before a credential group was picked (null) stayed cached after it was, and the
136+
// card kept showing the raw id. The key now carries the selector's OWN query key, which
137+
// names every context field its result depends on.
138+
const keyFor = (credentialGroupId?: string) =>
139+
dynamicSubBlockOptionKeys.detail('workspace-1', 'block-1', 'providerFilter', 'gmail', [
140+
'selectors',
141+
'workspace.credentialGroupProviders',
142+
'workspace-1',
143+
credentialGroupId ?? 'none',
144+
])
145+
146+
expect(keyFor(undefined)).not.toEqual(keyFor('group-1'))
147+
expect(keyFor('group-1')).not.toEqual(keyFor('group-2'))
148+
expect(keyFor('group-1')).toEqual(keyFor('group-1'))
149+
})
129150
})

apps/sim/hooks/queries/dynamic-subblock-options.ts

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useMemo } from 'react'
1+
import { useMemo } from 'react'
22
import { useQueries } from '@tanstack/react-query'
33
import { buildSelectorContextFromBlock } from '@/lib/workflows/subblocks/context'
44
import { summarizeNames } from '@/lib/workflows/subblocks/display'
@@ -14,13 +14,26 @@ export const DYNAMIC_SUBBLOCK_OPTION_STALE_TIME = 30 * 1000
1414
export const dynamicSubBlockOptionKeys = {
1515
all: ['dynamic-subblock-options'] as const,
1616
details: () => [...dynamicSubBlockOptionKeys.all, 'detail'] as const,
17-
detail: (workspaceId?: string, blockId?: string, subBlockId?: string, optionId?: string) =>
17+
/**
18+
* `selectorScope` is the selector's OWN query key for this context — every context field its
19+
* result depends on, named by the selector rather than restated here. Without it a label
20+
* resolved under an empty or previous sibling (no credential group picked yet) stays cached
21+
* and is reused once the sibling is set, so the card keeps showing a raw id or a stale name.
22+
*/
23+
detail: (
24+
workspaceId?: string,
25+
blockId?: string,
26+
subBlockId?: string,
27+
optionId?: string,
28+
selectorScope: readonly unknown[] = []
29+
) =>
1830
[
1931
...dynamicSubBlockOptionKeys.details(),
2032
workspaceId ?? '',
2133
blockId ?? '',
2234
subBlockId ?? '',
2335
optionId ?? '',
36+
...selectorScope,
2437
] as const,
2538
}
2639

@@ -60,34 +73,54 @@ export function useDynamicSubBlockOptionDisplayName({
6073
* silently fails every selector scoped by a sibling — `workspace.credentialGroupProviders`
6174
* needs the group before it can name a provider, so the card fell back to raw ids.
6275
*/
63-
const buildResolverContext = useCallback((): SelectorContext => {
64-
const activeWorkflowId = useWorkflowRegistry.getState().activeWorkflowId
65-
const block = blockId ? useWorkflowStore.getState().blocks[blockId] : undefined
66-
if (!block?.type || !blockId) return { workspaceId }
67-
const live = activeWorkflowId
68-
? (useSubBlockStore.getState().workflowValues[activeWorkflowId]?.[blockId] ?? {})
69-
: {}
76+
const activeWorkflowId = useWorkflowRegistry((state) => state.activeWorkflowId)
77+
const block = useWorkflowStore((state) => (blockId ? state.blocks[blockId] : undefined))
78+
const liveValues = useSubBlockStore((state) =>
79+
activeWorkflowId && blockId ? state.workflowValues[activeWorkflowId]?.[blockId] : undefined
80+
)
81+
82+
const resolverContext = useMemo((): SelectorContext => {
83+
if (!block?.type) return { workspaceId }
7084
const merged: Record<string, { value?: unknown }> = { ...(block.subBlocks ?? {}) }
71-
for (const [id, value] of Object.entries(live)) merged[id] = { ...merged[id], value }
85+
for (const [id, value] of Object.entries(liveValues ?? {})) {
86+
merged[id] = { ...merged[id], value }
87+
}
7288
return buildSelectorContextFromBlock(block.type, merged, {
7389
workflowId: activeWorkflowId ?? undefined,
7490
workspaceId,
7591
canonicalModes: block.data?.canonicalModes,
7692
})
77-
}, [blockId, workspaceId])
93+
}, [block, liveValues, activeWorkflowId, workspaceId])
94+
95+
/**
96+
* The selector's own key for this context. Reusing it means the cache is scoped by exactly
97+
* what the selector reads — no second list of context fields to keep in step, and it stays
98+
* correct when a selector's dependencies change.
99+
*/
100+
const selectorScope = useMemo(
101+
() =>
102+
definition ? definition.getQueryKey({ key: definition.key, context: resolverContext }) : [],
103+
[definition, resolverContext]
104+
)
78105
const canResolve = Boolean(blockId && fetchById && optionIds.length > 0)
79106

80107
const queries = useQueries({
81108
queries: canResolve
82109
? optionIds.map((optionId) => ({
83-
queryKey: dynamicSubBlockOptionKeys.detail(workspaceId, blockId, subBlock?.id, optionId),
110+
queryKey: dynamicSubBlockOptionKeys.detail(
111+
workspaceId,
112+
blockId,
113+
subBlock?.id,
114+
optionId,
115+
selectorScope as readonly unknown[]
116+
),
84117
queryFn: ({ signal }) => {
85118
if (!blockId || !fetchById || !definition) {
86119
throw new Error('Dynamic subblock option resolver is required')
87120
}
88121
return fetchById({
89122
key: definition.key,
90-
context: buildResolverContext(),
123+
context: resolverContext,
91124
detailId: optionId,
92125
signal,
93126
})

0 commit comments

Comments
 (0)