From d65db2378708149c4cdcaa95085283be5335fbd0 Mon Sep 17 00:00:00 2001
From: Diego Imbert <diego@windmill.dev>
Date: Fri, 15 May 2026 10:39:03 +0200
Subject: [PATCH] refactor(frontend): replace UserDraft.release() with
 useMany()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Public surface change:
- New `UserDraft.useMany(getSpecs: () => UserDraftSpec<V>[])` returns a
  reactive array of handles. The reconcile loop acquires entries for
  added specs, releases entries for removed specs, and re-uses cached
  handles for unchanged keys so caller-captured references stay stable.
- `UserDraft.use(kind, path, opts?)` becomes a 1-len wrapper around
  `useMany`. The spec getter is `untrack`ed so reactive opts
  (`$workspaceStore` etc.) are still captured-once — current `use()`
  semantics unchanged.
- `UserDraftHandle.release()` and the `manualRelease` option are gone.
  Component teardown is handled by a single internal `onDestroy` that
  releases every entry `useMany` acquired.

ResourceEditor + VariableEditor migrated:
- Replaced `Record<ws, Handle>` + manual `ensureHandle`/`release` with
  a `workspaceSpecs: $state<Array<{ws, defaultValue}>>` plus a
  derived `Record<ws, Handle>` that pairs each ws with its parallel
  handle from `useMany`. `ensureHandle(ws)` is now just a push to
  the specs array; `VariableEditor.reset()` clears it. The reconcile
  loop handles acquisition/release end-to-end.

Tests:
- Dropped the `manualRelease`/`release` test; the option no longer
  exists.
- Added a `useMany` test asserting per-spec entries, isolated
  workspace-scoped localStorage keys, and a single onDestroy
  registration covering every acquired entry.

Implementation note: I tried wrapping `useLocalStorageValue` in
`$effect.root` to give the entry's `$state`/`$effect` an independent
scope (in case `useMany`'s reconcile effect tore down nested effects
across cycles). But `$effect.root`'s callback wasn't running
synchronously in the test runtime (vitest + svelte-vite plugin), and
the original `use()` implementation called `useLocalStorageValue`
directly without issue. Reverted to the direct call; the
nested-scope concern stays theoretical.
---
 .../src/lib/components/ResourceEditor.svelte  |  49 ++--
 .../src/lib/components/VariableEditor.svelte  |  54 ++--
 frontend/src/lib/userDraft.svelte.ts          | 253 ++++++++++++------
 frontend/src/lib/userDraft.test.ts            |  39 +--
 4 files changed, 254 insertions(+), 141 deletions(-)

diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte
index 19e55138c2..6827eb74f5 100644
--- a/frontend/src/lib/components/ResourceEditor.svelte
+++ b/frontend/src/lib/components/ResourceEditor.svelte
@@ -2,7 +2,7 @@
 	import type { Schema } from '$lib/common'
 	import { ResourceService, WorkspaceService, type Resource, type ResourceType } from '$lib/gen'
 	import { canWrite } from '$lib/utils'
-	import { createEventDispatcher, onDestroy, untrack } from 'svelte'
+	import { createEventDispatcher, untrack } from 'svelte'
 	import { userStore, workspaceStore } from '$lib/stores'
 	import { sendUserToast } from '$lib/toast'
 	import { clearJsonSchemaResourceCache } from './schema/jsonSchemaResource.svelte'
@@ -49,32 +49,41 @@
 	let effectiveWorkspace = $derived(workspace ?? $workspaceStore!)
 	let initialPath = path
 
-	// Per-workspace handles. Each workspace's autosave lives at its own
-	// localStorage key (`userdraft/w/{ws}/resource/{initialPath}`) so editing
-	// the same path across two workspaces stays cleanly separated.
-	let states: Record<string, UserDraftHandle<ResourceState>> = $state({})
+	// Per-workspace handles are driven by `useMany`. We track the workspace
+	// IDs (and their seeded defaults) in a parallel `$state` array; on every
+	// mutation `useMany` reconciles, acquiring entries for new workspaces and
+	// releasing them on component teardown. `states` indexes the resulting
+	// handles by workspace ID for ergonomic lookup downstream.
+	let workspaceSpecs = $state<Array<{ ws: string; defaultValue: ResourceState }>>([])
 	let initialStates: Record<string, ResourceState> = $state({})
 	let existedInitially: Record<string, boolean> = $state({})
 	let fetchedResources: Record<string, Resource> = $state({})
 	let perWsUser: Record<string, UserExt | undefined> = $state({})
 
-	onDestroy(() => {
-		for (const h of Object.values(states)) h.release()
+	const handlesArray = UserDraft.useMany<ResourceState>(() =>
+		workspaceSpecs.map((s) => ({
+			itemKind: 'resource' as const,
+			path: initialPath ?? '',
+			workspace: s.ws,
+			defaultValue: s.defaultValue
+		}))
+	)
+	const states = $derived.by(() => {
+		const out: Record<string, UserDraftHandle<ResourceState>> = {}
+		for (let i = 0; i < workspaceSpecs.length; i++) {
+			const handle = handlesArray[i]
+			if (handle) out[workspaceSpecs[i].ws] = handle
+		}
+		return out
 	})
 
-	/** Create (or reuse) a per-workspace handle. `defaultValue` is what the
-	 * handle reports when no autosave is persisted; an existing autosave
-	 * always wins. The default itself never round-trips to localStorage — only
-	 * the user's first real edit triggers a write. */
-	function ensureHandle(ws: string, defaultValue: ResourceState): UserDraftHandle<ResourceState> {
-		if (states[ws]) return states[ws]
-		const h = UserDraft.use<ResourceState>('resource', initialPath ?? '', {
-			workspace: ws,
-			defaultValue,
-			manualRelease: true
-		})
-		states[ws] = h
-		return h
+	/** Register a workspace so `useMany` acquires (or reuses) its handle.
+	 * `defaultValue` is what the handle reports when no autosave is persisted;
+	 * an existing autosave always wins. The default itself never round-trips
+	 * to localStorage — only the user's first real edit triggers a write. */
+	function ensureHandle(ws: string, defaultValue: ResourceState): void {
+		if (workspaceSpecs.some((s) => s.ws === ws)) return
+		workspaceSpecs.push({ ws, defaultValue })
 	}
 
 	let isValid = $state(true)
diff --git a/frontend/src/lib/components/VariableEditor.svelte b/frontend/src/lib/components/VariableEditor.svelte
index 5d3a9b3f06..3ab1d71fc9 100644
--- a/frontend/src/lib/components/VariableEditor.svelte
+++ b/frontend/src/lib/components/VariableEditor.svelte
@@ -1,6 +1,6 @@
 <script lang="ts">
 	import { VariableService, WorkspaceService } from '$lib/gen'
-	import { createEventDispatcher, onDestroy, untrack } from 'svelte'
+	import { createEventDispatcher, untrack } from 'svelte'
 	import { userStore, workspaceStore } from '$lib/stores'
 	import { Button } from './common'
 	import Drawer from './common/drawer/Drawer.svelte'
@@ -28,10 +28,12 @@
 
 	let editPath: string | undefined = $state(undefined)
 
-	// Per-workspace handles. Each workspace's autosave lives at its own
-	// localStorage key (`userdraft/w/{ws}/variable/{editPath}`) so editing the
-	// same path across two workspaces stays cleanly separated.
-	let states: Record<string, UserDraftHandle<VariableState>> = $state({})
+	// Per-workspace handles are driven by `useMany`. We track the workspace
+	// IDs (and their seeded defaults) in a parallel `$state` array; on every
+	// mutation `useMany` reconciles, acquiring entries for new workspaces and
+	// releasing them on component teardown. `states` indexes the resulting
+	// handles by workspace ID for ergonomic lookup downstream.
+	let workspaceSpecs = $state<Array<{ ws: string; defaultValue: VariableState }>>([])
 	let initialStates: Record<string, VariableState> = $state({})
 	let existedInitially: Record<string, boolean> = $state({})
 	let extraPerms: Record<string, Record<string, boolean>> = $state({})
@@ -39,23 +41,30 @@
 	let selected: string | undefined = $state(undefined)
 	let pathError = $state('')
 
-	onDestroy(() => {
-		for (const h of Object.values(states)) h.release()
+	const handlesArray = UserDraft.useMany<VariableState>(() =>
+		workspaceSpecs.map((s) => ({
+			itemKind: 'variable' as const,
+			path: editPath ?? '',
+			workspace: s.ws,
+			defaultValue: s.defaultValue
+		}))
+	)
+	const states = $derived.by(() => {
+		const out: Record<string, UserDraftHandle<VariableState>> = {}
+		for (let i = 0; i < workspaceSpecs.length; i++) {
+			const handle = handlesArray[i]
+			if (handle) out[workspaceSpecs[i].ws] = handle
+		}
+		return out
 	})
 
-	/** Create (or reuse) a per-workspace handle. `defaultValue` is what the
-	 * handle reports when no autosave is persisted; an existing autosave
-	 * always wins. The default itself never round-trips to localStorage — only
-	 * the user's first real edit triggers a write. */
-	function ensureHandle(ws: string, defaultValue: VariableState): UserDraftHandle<VariableState> {
-		if (states[ws]) return states[ws]
-		const h = UserDraft.use<VariableState>('variable', editPath ?? '', {
-			workspace: ws,
-			defaultValue,
-			manualRelease: true
-		})
-		states[ws] = h
-		return h
+	/** Register a workspace so `useMany` acquires (or reuses) its handle.
+	 * `defaultValue` is what the handle reports when no autosave is persisted;
+	 * an existing autosave always wins. The default itself never round-trips
+	 * to localStorage — only the user's first real edit triggers a write. */
+	function ensureHandle(ws: string, defaultValue: VariableState): void {
+		if (workspaceSpecs.some((s) => s.ws === ws)) return
+		workspaceSpecs.push({ ws, defaultValue })
 	}
 
 	let drawer: Drawer | undefined = $state()
@@ -130,8 +139,9 @@
 	})
 
 	function reset() {
-		for (const h of Object.values(states)) h.release()
-		states = {}
+		// Clearing workspaceSpecs triggers useMany's reconcile to release
+		// every acquired entry. The $derived `states` then collapses to {}.
+		workspaceSpecs = []
 		initialStates = {}
 		existedInitially = {}
 		extraPerms = {}
diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts
index 00fb713bca..69ce219de5 100644
--- a/frontend/src/lib/userDraft.svelte.ts
+++ b/frontend/src/lib/userDraft.svelte.ts
@@ -1,5 +1,5 @@
 import { get } from 'svelte/store'
-import { onDestroy } from 'svelte'
+import { onDestroy, untrack } from 'svelte'
 import { workspaceStore } from './stores'
 import { useLocalStorageValue } from './svelte5Utils.svelte'
 
@@ -40,14 +40,18 @@ export type UserDraftUseOptions<V> = UserDraftOptions & {
 	 * actual mutation is what writes to localStorage.
 	 */
 	defaultValue?: V
-	/**
-	 * When `true`, skip the automatic `onDestroy` registration. The caller is
-	 * responsible for invoking `handle.release()` to decrement the entry's
-	 * refcount. Useful when handles are created dynamically (e.g. inside an
-	 * effect) where `onDestroy` can no longer be called — Svelte 5 only
-	 * accepts lifecycle hooks during component initialization.
-	 */
-	manualRelease?: boolean
+}
+
+/**
+ * A single (kind, path, workspace) tuple that `useMany` should hold a handle
+ * for. The shape mirrors `use()`'s arguments, just bundled into one object
+ * so a getter can return a list of them.
+ */
+export type UserDraftSpec<V> = {
+	itemKind: UserDraftItemKind
+	path: string
+	workspace?: string
+	defaultValue?: V
 }
 
 /**
@@ -216,12 +220,6 @@ export type UserDraftHandle<V> = {
 	 * just acknowledged it.
 	 */
 	setMeta(meta: UserDraftMeta, opts?: { force?: boolean }): void
-	/**
-	 * Manually decrement the entry's refcount. Only required when `use()` was
-	 * called with `manualRelease: true` — otherwise an `onDestroy` is already
-	 * wired in. Calling this more than once for the same handle is a no-op.
-	 */
-	release(): void
 }
 
 export const UserDraft = {
@@ -332,73 +330,166 @@ export const UserDraft = {
 		path: string,
 		opts?: UserDraftUseOptions<V>
 	): UserDraftHandle<V> {
-		const ws = resolveWorkspace(opts)
-		const mk = mapKey(ws, itemKind, path)
-		const wrappedDefault = wrap(opts?.defaultValue)
-
-		let entry = entries.get(mk)
-		if (!entry) {
-			const state = useLocalStorageValue<StoredDraft<unknown> | undefined>(
-				localStorageKey(ws, itemKind, path),
-				wrappedDefault,
-				undefined,
-				// The first value to flow into the handle (e.g. a backend load
-				// in the editor route) is the baseline — only persist when the
-				// user actually changes it afterwards. Coalesce a typing storm
-				// into one localStorage write per 500 ms.
-				{ saveInitialValue: false, debounce: 500 }
-			)
-			entry = { count: 1, state }
-			entries.set(mk, entry)
-		} else {
-			entry.count++
-		}
-
-		const sharedEntry = entry
-
-		let released = false
-		const release = (): void => {
-			if (released) return
-			released = true
-			const e = entries.get(mk)
-			if (!e) return
-			e.count--
-			if (e.count <= 0) {
-				entries.delete(mk)
-			}
-		}
-
-		if (!opts?.manualRelease) {
-			onDestroy(release)
-		}
-
-		return {
-			get draft(): V | undefined {
-				return unwrap(sharedEntry.state.val as StoredDraft<V> | undefined)
-			},
-			set draft(value: V | undefined) {
-				// Preserve existing rev metadata when the user just edits the
-				// value (e.g. typing in the editor). useLocalStorageValue's
-				// setter writes synchronously and removes the localStorage
-				// entry when value is undefined.
-				const current = sharedEntry.state.val as StoredDraft<V> | undefined
-				sharedEntry.state.val = wrap(value, extractMeta(current))
-			},
-			get meta(): UserDraftMeta {
-				return extractMeta(sharedEntry.state.val as StoredDraft<unknown> | undefined)
-			},
-			setDraftAndMeta(value: V | undefined, meta: UserDraftMeta): void {
-				sharedEntry.state.val = wrap(value, meta)
-			},
-			setMeta(meta: UserDraftMeta, opts?: { force?: boolean }): void {
-				const current = sharedEntry.state.val as StoredDraft<V> | undefined
-				if (current === undefined) return
-				sharedEntry.state.val = wrap(current.value, meta)
-				if (opts?.force) {
-					persistDirect(localStorageKey(ws, itemKind, path), current.value, meta)
+		// `use()` is a single-spec wrapper around `useMany`. We untrack the
+		// getter so that reactive opts (e.g. `$workspaceStore`) are captured
+		// once at call time — the current `use()` contract is "the handle
+		// stays bound to this workspace until the component unmounts." Use
+		// `useMany` directly if you want spec changes to release/acquire
+		// entries as you go.
+		const handles = UserDraft.useMany<V>(() =>
+			untrack(() => [
+				{
+					itemKind,
+					path,
+					workspace: opts?.workspace,
+					defaultValue: opts?.defaultValue
 				}
-			},
-			release
+			])
+		)
+		return handles[0]
+	},
+
+	useMany<V = unknown>(getSpecs: () => UserDraftSpec<V>[]): UserDraftHandle<V>[] {
+		// Reactive handles array, reconciled against the latest `getSpecs()`
+		// output. Indices line up with the spec array. Handles for the same
+		// (workspace, kind, path) tuple are reused across reconciles so
+		// callers can capture a reference and keep it alive — only the
+		// underlying entry's refcount moves.
+		const handles = $state<UserDraftHandle<V>[]>([])
+		const acquired = new Set<string>()
+		const handleCache = new Map<string, UserDraftHandle<V>>()
+
+		function reconcile() {
+			const specs = getSpecs()
+			const seen = new Set<string>()
+			const next: UserDraftHandle<V>[] = []
+
+			for (const spec of specs) {
+				const ws = spec.workspace ?? resolveWorkspace()
+				const mk = mapKey(ws, spec.itemKind, spec.path)
+				seen.add(mk)
+
+				if (!acquired.has(mk)) {
+					acquireEntry(ws, spec.itemKind, spec.path, spec.defaultValue)
+					acquired.add(mk)
+				}
+				let handle = handleCache.get(mk)
+				if (!handle) {
+					handle = makeHandle<V>(ws, spec.itemKind, spec.path)
+					handleCache.set(mk, handle)
+				}
+				next.push(handle)
+			}
+
+			for (const mk of [...acquired]) {
+				if (!seen.has(mk)) {
+					releaseEntry(mk)
+					acquired.delete(mk)
+					handleCache.delete(mk)
+				}
+			}
+
+			// Skip the mutation when specs are structurally unchanged — handles
+			// are cached by mapKey, so two reconciles with the same spec set
+			// produce reference-equal arrays. Avoids dirtying downstream
+			// reactive readers on no-op `$effect` re-runs.
+			const unchanged = handles.length === next.length && handles.every((h, i) => h === next[i])
+			if (!unchanged) handles.splice(0, handles.length, ...next)
+		}
+
+		// Synchronous initial reconcile so single-spec callers (`use()`) get a
+		// populated `handles[0]` before the function returns. Reactive reads
+		// inside `getSpecs()` here are intentionally not tracked — the
+		// `$effect` below picks up any subsequent dependency changes.
+		untrack(reconcile)
+		$effect(reconcile)
+		onDestroy(() => {
+			for (const mk of acquired) releaseEntry(mk)
+			acquired.clear()
+			handleCache.clear()
+		})
+
+		return handles
+	}
+}
+
+function acquireEntry(
+	workspace: string,
+	itemKind: UserDraftItemKind,
+	path: string,
+	defaultValue: unknown
+): void {
+	const mk = mapKey(workspace, itemKind, path)
+	const existing = entries.get(mk)
+	if (existing) {
+		existing.count++
+		return
+	}
+	const state = useLocalStorageValue<StoredDraft<unknown> | undefined>(
+		localStorageKey(workspace, itemKind, path),
+		wrap(defaultValue),
+		undefined,
+		// The first value to flow into the handle (e.g. a backend load in
+		// the editor route) is the baseline — only persist when the user
+		// actually changes it afterwards. Coalesce a typing storm into one
+		// localStorage write per 500 ms.
+		{ saveInitialValue: false, debounce: 500 }
+	)
+	entries.set(mk, { count: 1, state })
+}
+
+function releaseEntry(mk: string): void {
+	const entry = entries.get(mk)
+	if (!entry) return
+	entry.count--
+	if (entry.count <= 0) {
+		entries.delete(mk)
+	}
+}
+
+function makeHandle<V>(
+	workspace: string,
+	itemKind: UserDraftItemKind,
+	path: string
+): UserDraftHandle<V> {
+	// The handle reads `entries.get(mk)` on every access. The entry it points
+	// at is stable as long as the refcount stays > 0 (which `useMany` keeps
+	// the case for as long as a spec references it). If the refcount drops to
+	// 0 and the entry is destroyed, reads return `undefined` rather than
+	// throwing — the consumer should already have been torn down by that point.
+	const mk = mapKey(workspace, itemKind, path)
+	const stateOf = (): DraftState<unknown> | undefined => entries.get(mk)?.state
+	return {
+		get draft(): V | undefined {
+			return unwrap(stateOf()?.val as StoredDraft<V> | undefined)
+		},
+		set draft(value: V | undefined) {
+			// Preserve existing rev metadata when the user just edits the
+			// value (e.g. typing in the editor). useLocalStorageValue's
+			// setter writes synchronously and removes the localStorage
+			// entry when value is undefined.
+			const state = stateOf()
+			if (!state) return
+			const current = state.val as StoredDraft<V> | undefined
+			state.val = wrap(value, extractMeta(current))
+		},
+		get meta(): UserDraftMeta {
+			return extractMeta(stateOf()?.val as StoredDraft<unknown> | undefined)
+		},
+		setDraftAndMeta(value: V | undefined, meta: UserDraftMeta): void {
+			const state = stateOf()
+			if (!state) return
+			state.val = wrap(value, meta)
+		},
+		setMeta(meta: UserDraftMeta, opts?: { force?: boolean }): void {
+			const state = stateOf()
+			if (!state) return
+			const current = state.val as StoredDraft<V> | undefined
+			if (current === undefined) return
+			state.val = wrap(current.value, meta)
+			if (opts?.force) {
+				persistDirect(localStorageKey(workspace, itemKind, path), current.value, meta)
+			}
 		}
 	}
 }
diff --git a/frontend/src/lib/userDraft.test.ts b/frontend/src/lib/userDraft.test.ts
index b41301182c..69a26ed95d 100644
--- a/frontend/src/lib/userDraft.test.ts
+++ b/frontend/src/lib/userDraft.test.ts
@@ -511,26 +511,29 @@ describe('UserDraft.use() — reference counting & cleanup', () => {
 		vi.advanceTimersByTime(500)
 		expect(localStorage.getItem('userdraft/w/test_ws/flow/u/me/debounce')).toBe(wrapped('three'))
 	})
+})
 
-	it('manualRelease=true skips onDestroy registration and gates cleanup behind handle.release()', () => {
-		// Caller opts out of the auto-onDestroy — used by routes that create
-		// handles dynamically (e.g. ResourceEditor's per-workspace handles).
-		const h = UserDraft.use<string>('flow', 'u/me/manual', { manualRelease: true })
-		expect(onDestroyCallbacks.length).toBe(0)
-		h.draft = 'initial' // baseline
-		h.draft = 'edited' // persisted
+describe('UserDraft.useMany()', () => {
+	it('acquires one handle per spec in the synchronous initial reconcile', () => {
+		// `useMany`'s sync reconcile populates handles[0..] before returning,
+		// so callers (and `use()`'s 1-len wrapper) can use them immediately
+		// without waiting for an `$effect` tick.
+		const handles = UserDraft.useMany<number>(() => [
+			{ itemKind: 'flow', path: 'u/me/many', workspace: 'a' },
+			{ itemKind: 'flow', path: 'u/me/many', workspace: 'b' }
+		])
+		expect(handles.length).toBe(2)
 
-		// Without release(), the entry stays alive — a co-resident handle
-		// sees the same in-memory state.
-		const h2 = UserDraft.use<string>('flow', 'u/me/manual', { manualRelease: true })
-		expect(h2.draft).toBe('edited')
-		h.release()
-		// h2 still holds the entry. Releasing both clears it.
-		UserDraft.save('flow', 'u/me/manual', 'edited2')
-		expect(h2.draft).toBe('edited2')
+		// Each spec gets its own entry in the workspace-keyed store.
+		handles[0].draft = 0 // baseline
+		handles[0].draft = 1 // persisted
+		handles[1].draft = 0
+		handles[1].draft = 9
+		flushPersist()
+		expect(localStorage.getItem('userdraft/w/a/flow/u/me/many')).toBe(wrapped(1))
+		expect(localStorage.getItem('userdraft/w/b/flow/u/me/many')).toBe(wrapped(9))
 
-		h2.release()
-		// Second release on the same handle is a no-op — refcount stays at 0.
-		h2.release()
+		// One component-level onDestroy releases every acquired entry.
+		expect(onDestroyCallbacks.length).toBe(1)
 	})
 })