refactor(frontend): replace UserDraft.release() with useMany()

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.
This commit is contained in:
Diego Imbert
2026-05-15 10:39:03 +02:00
parent 5c2209dc93
commit d65db23787
4 changed files with 254 additions and 141 deletions
@@ -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)
@@ -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 = {}
+172 -81
View File
@@ -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)
}
}
}
}
+21 -18
View File
@@ -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)
})
})