mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-12 16:05:43 +00:00
make selected resilient + snapshot args for React (#9298)
* fix(ResourceEditor): make `selected` resilient + snapshot args for React Two issues surfaced via the React SDK (reactify wrapper re-spreads Svelte props on every host re-render): 1. The bindable `selected` prop transiently resets to undefined on each re-spread, flipping `current` through undefined and unmounting the form (input loses focus on every keystroke). Rename the prop to `selectedProp` and derive `selected = selectedProp ?? effectiveWorkspace` so the fallback insulates the component without effects. 2. The onChange dispatch passed `current.args` (a `$state` proxy) directly, so React consumers diffing by reference or JSON.stringify saw the same value forever, and the effect only tracked the args reference (not nested mutations). Wrap with `$state.snapshot` to deep-track and emit a plain object. The bootstrap effect is also restructured: it no longer writes `selected` (the derived handles defaulting) and now guards on `selected in initialStates` so workspace flips remain idempotent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ResourceEditor): declare effectiveWorkspace before use in selected Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
tristantr
co-authored by
Claude Opus 4.7
parent
79d2f01d02
commit
f6f3781ef5
@@ -36,7 +36,7 @@
|
||||
onChange,
|
||||
defaultValues = undefined,
|
||||
workspace = undefined,
|
||||
selected = $bindable()
|
||||
selected: selectedProp = $bindable()
|
||||
}: Props = $props()
|
||||
|
||||
type ResourceState = {
|
||||
@@ -50,6 +50,10 @@
|
||||
const dispatch = createEventDispatcher()
|
||||
|
||||
let effectiveWorkspace = $derived(workspace ?? $workspaceStore!)
|
||||
// Fallback to `effectiveWorkspace` insulates against reactify-style
|
||||
// parents that re-spread props without `selected` — otherwise it
|
||||
// transiently resets and the form below remounts on every keystroke.
|
||||
let selected = $derived(selectedProp ?? effectiveWorkspace)
|
||||
let initialPath = path
|
||||
|
||||
// Per-workspace handles are driven by `useMany`. We track the workspace
|
||||
@@ -205,28 +209,25 @@
|
||||
})
|
||||
)
|
||||
|
||||
// Bootstrap: ensure selected is set on mount (edit or new)
|
||||
// New-resource bootstrap: seed empty state per workspace (edit mode
|
||||
// is seeded by the lazy-fetch effect below).
|
||||
$effect(() => {
|
||||
selected
|
||||
if (!effectiveWorkspace) return
|
||||
if (!selected) return
|
||||
if (initialPath) return
|
||||
if (selected in initialStates) return
|
||||
untrack(() => {
|
||||
if (selected !== undefined) return
|
||||
selected = effectiveWorkspace
|
||||
if (!initialPath) {
|
||||
// New resource
|
||||
const s: ResourceState = {
|
||||
path: '',
|
||||
description: '',
|
||||
args: (defaultValues && Object.keys(defaultValues).length > 0
|
||||
? defaultValues
|
||||
: {}) as any,
|
||||
labels: undefined,
|
||||
wsSpecific: false
|
||||
}
|
||||
ensureHandle(effectiveWorkspace, s)
|
||||
initialStates[effectiveWorkspace] = structuredClone(s)
|
||||
existedInitially[effectiveWorkspace] = false
|
||||
const s: ResourceState = {
|
||||
path: '',
|
||||
description: '',
|
||||
args: (defaultValues && Object.keys(defaultValues).length > 0
|
||||
? defaultValues
|
||||
: {}) as any,
|
||||
labels: undefined,
|
||||
wsSpecific: false
|
||||
}
|
||||
ensureHandle(selected, s)
|
||||
initialStates[selected] = structuredClone(s)
|
||||
existedInitially[selected] = false
|
||||
})
|
||||
})
|
||||
|
||||
@@ -330,11 +331,9 @@
|
||||
|
||||
$effect(() => {
|
||||
if (current)
|
||||
// Snapshot args so the deep read establishes nested dependency
|
||||
// tracking (the effect re-runs on mutations inside args, not
|
||||
// just reference changes) and so consumers receive a plain
|
||||
// object instead of a $state proxy — important for React
|
||||
// integrations that diff by reference or JSON.stringify.
|
||||
// $state.snapshot deep-reads (so the effect re-runs on nested
|
||||
// args mutations) and returns a plain object (React consumers
|
||||
// can't diff a $state proxy by reference or JSON.stringify).
|
||||
onChange?.({
|
||||
path: current.path,
|
||||
args: $state.snapshot(current.args) as Record<string, any>,
|
||||
|
||||
Reference in New Issue
Block a user