mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(frontend): settle new-item drafts before the drawer closes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wy24UHSVRZdDaPWiBay9MG
This commit is contained in:
co-authored by
Claude Opus 5
parent
81a0773edf
commit
ed1ccc05b9
@@ -73,9 +73,14 @@
|
||||
|
||||
<Drawer
|
||||
bind:this={drawer}
|
||||
on:close={() => {
|
||||
on:close={async () => {
|
||||
// Flushed before the step reset (which retires the form the draft mirrors)
|
||||
// and before `close`, whose list refetch would otherwise outrun the draft's
|
||||
// own commit delay and the syncer's debounce, and miss the new row.
|
||||
const flushed = appConnectInner?.flushDraft()
|
||||
step = 1
|
||||
handedOff = false
|
||||
await flushed
|
||||
dispatch('close')
|
||||
}}
|
||||
size="700px"
|
||||
|
||||
@@ -993,7 +993,9 @@
|
||||
}
|
||||
})
|
||||
}
|
||||
newDraftSync.finish()
|
||||
// Awaited: `refresh` refetches the list, and a debounced delete would
|
||||
// leave the just-created resource still flagged as a draft.
|
||||
await newDraftSync.finish()
|
||||
dispatch('refresh', path)
|
||||
dispatch('close')
|
||||
sendUserToast(
|
||||
@@ -1005,10 +1007,16 @@
|
||||
}
|
||||
}
|
||||
|
||||
/** Settle the pending draft write before the drawer's close event, whose
|
||||
* list refetch would otherwise outrun the debounced POST. */
|
||||
export async function flushDraft(): Promise<void> {
|
||||
await newDraftSync.flush()
|
||||
}
|
||||
|
||||
export async function back() {
|
||||
if (step == 2 && manual) {
|
||||
// Back abandons this form; the draft it mirrored goes with it.
|
||||
newDraftSync.finish()
|
||||
await newDraftSync.finish()
|
||||
newDraftSync.reset()
|
||||
pathDirty = false
|
||||
}
|
||||
|
||||
@@ -331,6 +331,32 @@
|
||||
export function localDraftCurrent(): ResourceState | undefined {
|
||||
return current
|
||||
}
|
||||
/** A draft-only item's list row is keyed by the path INSIDE the draft, while
|
||||
* the autosave handle stays keyed on the path the editor opened. Renaming
|
||||
* one would leave the row pointing at a key that holds no draft — it would
|
||||
* 404 on reopen and its delete would miss — so move the draft to the path
|
||||
* the form now carries. Reads its state synchronously: the caller runs this
|
||||
* as the drawer closes, and awaiting first would race the editor's teardown. */
|
||||
function moveRenamedDraftOnly(): Promise<unknown> {
|
||||
const ws = selected
|
||||
if (!ws || !initialPath || existedInitially[ws] !== false) return Promise.resolve()
|
||||
const s = states[ws]?.draft
|
||||
if (!s || !s.path || s.path === initialPath) return Promise.resolve()
|
||||
UserDraft.save('resource', s.path, $state.snapshot(s) as ResourceState, { workspace: ws })
|
||||
UserDraft.remove('resource', initialPath, { workspace: ws })
|
||||
return Promise.all([
|
||||
UserDraftDbSyncer.flush({ workspace: ws, itemKind: 'resource', path: s.path }),
|
||||
UserDraftDbSyncer.flush({ workspace: ws, itemKind: 'resource', path: initialPath })
|
||||
])
|
||||
}
|
||||
|
||||
/** Settle every pending draft write. The drawer awaits this before its
|
||||
* `onClose`, whose list refetch would otherwise outrun the debounced POST. */
|
||||
export async function flushDraft(): Promise<void> {
|
||||
// Both started before the first await so they read live editor state.
|
||||
await Promise.all([newDraftSync.flush(), moveRenamedDraftOnly()])
|
||||
}
|
||||
|
||||
/** Returns true when the item was draft-only: discarding deleted it
|
||||
* outright, so there is nothing left for the editor to show. */
|
||||
export async function discardLocalDraft(): Promise<boolean> {
|
||||
@@ -422,7 +448,9 @@
|
||||
// `remove`. See VariableEditor for the full rationale.
|
||||
UserDraft.discard('resource', initialPath, s, { workspace: ws })
|
||||
} else {
|
||||
newDraftSync.finish()
|
||||
// Awaited: the caller refetches the list right after, and a debounced
|
||||
// delete would leave the just-created item still flagged as a draft.
|
||||
await newDraftSync.finish()
|
||||
}
|
||||
// Path now exists server-side — drop the autocomplete cache so
|
||||
// it shows up immediately instead of after the 60s TTL.
|
||||
|
||||
@@ -49,6 +49,7 @@
|
||||
localDraftDeployed: () => unknown
|
||||
localDraftCurrent: () => unknown
|
||||
discardLocalDraft: () => Promise<boolean>
|
||||
flushDraft: () => Promise<void>
|
||||
}
|
||||
| undefined = $state(undefined)
|
||||
let hasLocalDraft = $state(false)
|
||||
@@ -101,8 +102,11 @@
|
||||
bind:this={drawer}
|
||||
size="50rem"
|
||||
{disableChatOffset}
|
||||
on:close={() => {
|
||||
on:close={async () => {
|
||||
clearPageDrawerAnchor(RESOURCES_PATH)
|
||||
// Before `onClose`: its list refetch would otherwise outrun the draft's
|
||||
// own commit delay and the syncer's debounce, and miss the new row.
|
||||
await resourceEditor?.flushDraft()
|
||||
onClose?.()
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -30,9 +30,9 @@
|
||||
initialPath: string
|
||||
hidePath?: boolean
|
||||
/** `Path`'s validation error (`''` when valid). */
|
||||
pathError?: string
|
||||
pathError: string
|
||||
/** Whether the user edited the path (as opposed to `Path`'s auto-filled name). */
|
||||
pathDirty?: boolean
|
||||
pathDirty: boolean
|
||||
labels: string[] | undefined
|
||||
description: string
|
||||
args: Record<string, any>
|
||||
@@ -57,8 +57,8 @@
|
||||
path = $bindable(),
|
||||
initialPath,
|
||||
hidePath = false,
|
||||
pathError = $bindable(''),
|
||||
pathDirty = $bindable(false),
|
||||
pathError = $bindable(),
|
||||
pathDirty = $bindable(),
|
||||
labels = $bindable(),
|
||||
description = $bindable(),
|
||||
args = $bindable(),
|
||||
|
||||
@@ -230,6 +230,33 @@
|
||||
})
|
||||
})
|
||||
|
||||
/** A draft-only item's list row is keyed by the path INSIDE the draft, while
|
||||
* the autosave handle stays keyed on the path the editor opened. Renaming
|
||||
* one would leave the row pointing at a key that holds no draft — it would
|
||||
* 404 on reopen and its delete would miss — so move the draft to the path
|
||||
* the form now carries. Reads its state synchronously: the caller runs this
|
||||
* as the drawer closes, and awaiting first would race the form's teardown. */
|
||||
function moveRenamedDraftOnly(): Promise<unknown> {
|
||||
const ws = selected
|
||||
if (!ws || !editPath || existedInitially[ws] !== false) return Promise.resolve()
|
||||
const s = states[ws]?.draft
|
||||
if (!s || !s.path || s.path === editPath) return Promise.resolve()
|
||||
const from = editPath
|
||||
UserDraft.save('variable', s.path, $state.snapshot(s) as VariableState, { workspace: ws })
|
||||
UserDraft.remove('variable', from, { workspace: ws })
|
||||
return Promise.all([
|
||||
UserDraftDbSyncer.flush({ workspace: ws, itemKind: 'variable', path: s.path }),
|
||||
UserDraftDbSyncer.flush({ workspace: ws, itemKind: 'variable', path: from })
|
||||
])
|
||||
}
|
||||
|
||||
/** Settle every pending draft write before the drawer's close event, whose
|
||||
* list refetch would otherwise outrun the debounced POST. */
|
||||
async function flushDraft(): Promise<void> {
|
||||
// Both started before the first await so they read live form state.
|
||||
await Promise.all([newDraftSync.flush(), moveRenamedDraftOnly()])
|
||||
}
|
||||
|
||||
function reset() {
|
||||
// Clearing workspaceSpecs triggers useMany's reconcile to release
|
||||
// every acquired entry. The $derived `states` then collapses to {}.
|
||||
@@ -328,7 +355,9 @@
|
||||
if (editPath) {
|
||||
UserDraft.discard('variable', editPath, s, { workspace: ws })
|
||||
} else {
|
||||
newDraftSync.finish()
|
||||
// Awaited: the caller refetches the list right after, and a debounced
|
||||
// delete would leave the just-created item still flagged as a draft.
|
||||
await newDraftSync.finish()
|
||||
}
|
||||
// Path now exists server-side — drop the autocomplete cache so
|
||||
// it shows up immediately instead of after the 60s TTL.
|
||||
@@ -346,10 +375,11 @@
|
||||
<Drawer
|
||||
bind:this={drawer}
|
||||
size="50rem"
|
||||
on:close={() => {
|
||||
on:close={async () => {
|
||||
clearPageDrawerAnchor(VARIABLES_PATH)
|
||||
// A new variable left unsaved persists as a draft-only row, which a
|
||||
// list only sees on refetch.
|
||||
// A new variable left unsaved persists as a draft-only row, which a list
|
||||
// only sees on refetch — settle the write before asking for that refetch.
|
||||
await flushDraft()
|
||||
dispatch('close')
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -26,7 +26,7 @@
|
||||
initialPath: string
|
||||
pathError: string
|
||||
/** Whether the user edited the path (as opposed to `Path`'s auto-filled name). */
|
||||
pathDirty?: boolean
|
||||
pathDirty: boolean
|
||||
variable: Variable
|
||||
labels: string[] | undefined
|
||||
wsSpecific: boolean
|
||||
@@ -42,7 +42,7 @@
|
||||
path = $bindable(),
|
||||
initialPath,
|
||||
pathError = $bindable(),
|
||||
pathDirty = $bindable(false),
|
||||
pathDirty = $bindable(),
|
||||
variable = $bindable(),
|
||||
labels = $bindable(),
|
||||
wsSpecific = $bindable(),
|
||||
|
||||
@@ -3,12 +3,16 @@ import { flushSync } from 'svelte'
|
||||
|
||||
const save = vi.fn()
|
||||
const remove = vi.fn()
|
||||
const flush = vi.fn(async () => {})
|
||||
vi.mock('$lib/userDraft.svelte', () => ({
|
||||
UserDraft: {
|
||||
save: (...a: unknown[]) => save(...a),
|
||||
remove: (...a: unknown[]) => remove(...a)
|
||||
}
|
||||
}))
|
||||
vi.mock('$lib/userDraftDbSyncer.svelte', () => ({
|
||||
UserDraftDbSyncer: { flush: (...a: unknown[]) => flush(...(a as [])) }
|
||||
}))
|
||||
|
||||
import { useNewItemDraftSync } from './useNewItemDraftSync.svelte'
|
||||
|
||||
@@ -18,8 +22,8 @@ afterEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
/** Drives the helper through a new-item drawer session and pins the writes
|
||||
* it must and must not make: nothing for an untouched form (the path field
|
||||
/** Drives the helper through a new-item drawer session and pins the writes it
|
||||
* must and must not make: nothing for an untouched form (the path field
|
||||
* auto-fills a name on mount), a move that deletes the key it left, a delete
|
||||
* once the path fails validation, and no delete at all on teardown — a draft
|
||||
* left behind by closing the drawer is the feature. */
|
||||
@@ -86,7 +90,44 @@ describe('useNewItemDraftSync', () => {
|
||||
expect(remove).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('finish deletes the persisted key and stops mirroring until reset', () => {
|
||||
/** The commit is delayed so the key can't land on a half-typed path, and the
|
||||
* editor is destroyed the moment its drawer closes. Closing right after the
|
||||
* first edit therefore tears down mid-delay, and the draft must survive it. */
|
||||
it('persists a commit still pending when the editor is torn down', async () => {
|
||||
const form = $state({ path: 'u/me/quick', touched: false, n: 1 })
|
||||
let sync: ReturnType<typeof useNewItemDraftSync> | undefined
|
||||
const cleanup = $effect.root(() => {
|
||||
sync = useNewItemDraftSync({
|
||||
itemKind: 'resource',
|
||||
enabled: () => true,
|
||||
workspace: () => 'w',
|
||||
path: () => form.path,
|
||||
pathError: () => '',
|
||||
touched: () => form.touched,
|
||||
value: () => ({ n: form.n })
|
||||
})
|
||||
})
|
||||
flushSync()
|
||||
form.touched = true
|
||||
flushSync()
|
||||
|
||||
// Torn down half-way through the commit delay.
|
||||
vi.advanceTimersByTime(500)
|
||||
cleanup()
|
||||
vi.advanceTimersByTime(500)
|
||||
expect(save).toHaveBeenCalledWith('resource', 'u/me/quick', { n: 1 }, { workspace: 'w' })
|
||||
|
||||
// The drawer's close handler awaits this, so the list refetch behind it
|
||||
// sees the row rather than racing the syncer's debounce.
|
||||
await sync!.flush()
|
||||
expect(flush).toHaveBeenCalledWith({
|
||||
workspace: 'w',
|
||||
itemKind: 'resource',
|
||||
path: 'u/me/quick'
|
||||
})
|
||||
})
|
||||
|
||||
it('finish deletes the persisted key and stops mirroring until reset', async () => {
|
||||
const form = $state({ path: 'u/me/item', touched: true, n: 1 })
|
||||
let sync: ReturnType<typeof useNewItemDraftSync> | undefined
|
||||
const cleanup = $effect.root(() => {
|
||||
@@ -105,8 +146,7 @@ describe('useNewItemDraftSync', () => {
|
||||
flushSync()
|
||||
expect(save).toHaveBeenCalledTimes(1)
|
||||
|
||||
sync!.finish()
|
||||
flushSync()
|
||||
await sync!.finish()
|
||||
expect(remove).toHaveBeenCalledWith('variable', 'u/me/item', { workspace: 'w' })
|
||||
|
||||
form.n = 2
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { untrack } from 'svelte'
|
||||
import { UserDraft, type UserDraftItemKind } from '$lib/userDraft.svelte'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
|
||||
/** Longer than `Path`'s 500ms debounced existence check, so the key never
|
||||
* lands on a half-typed path and the check's verdict is in before a commit. */
|
||||
@@ -27,8 +28,12 @@ export interface NewItemDraftSyncOptions<V> {
|
||||
export interface NewItemDraftSync {
|
||||
/** Storage path of the persisted draft, `''` when none. */
|
||||
readonly draftPath: string
|
||||
/** Commit anything still pending and settle it server-side. Callers MUST
|
||||
* await this before a list refetch (both the commit delay and the syncer's
|
||||
* own debounce outlive a closing drawer, so a refetch would miss the row). */
|
||||
flush(): Promise<void>
|
||||
/** Delete the persisted draft and stop mirroring, once the item is created. */
|
||||
finish(): void
|
||||
finish(): Promise<void>
|
||||
/** Re-arm for the next drawer session (an editor instance that outlives
|
||||
* its drawer). Forgets the previous session's key without deleting it: a
|
||||
* draft left behind by closing the drawer is the point. */
|
||||
@@ -46,50 +51,112 @@ export interface NewItemDraftSync {
|
||||
export function useNewItemDraftSync<V>(opts: NewItemDraftSyncOptions<V>): NewItemDraftSync {
|
||||
let draftPath = $state('')
|
||||
let finished = $state(false)
|
||||
// Last key actually written: a moved or finished draft deletes exactly the
|
||||
// row it left behind, and component teardown deletes nothing.
|
||||
let writtenPath = ''
|
||||
// The key last written, workspace included: a move or a delete has to target
|
||||
// the row actually left behind, not wherever the form points now.
|
||||
let written: { workspace: string; path: string } | undefined
|
||||
let writtenValue: string | undefined
|
||||
// The commit the timer will make, snapshotted at schedule time so it still
|
||||
// lands once the editor is gone: closing a drawer a keystroke after the
|
||||
// first edit must keep the draft, so a pending commit is never cancelled by
|
||||
// teardown — only superseded by a newer one, or consumed by `flush`.
|
||||
let pending:
|
||||
| { timer: ReturnType<typeof setTimeout>; key: string; value: V | undefined }
|
||||
| undefined
|
||||
let pendingWorkspace: string | undefined
|
||||
|
||||
function write(workspace: string | undefined, path: string, value: V | undefined): void {
|
||||
if (written && (written.path !== path || written.workspace !== workspace)) {
|
||||
UserDraft.remove(opts.itemKind, written.path, { workspace: written.workspace })
|
||||
written = undefined
|
||||
writtenValue = undefined
|
||||
}
|
||||
if (!workspace || !path || value === undefined) return
|
||||
const serialized = JSON.stringify(value)
|
||||
if (written && serialized === writtenValue) return
|
||||
UserDraft.save(opts.itemKind, path, value, { workspace })
|
||||
written = { workspace, path }
|
||||
writtenValue = serialized
|
||||
}
|
||||
|
||||
function dropPending(): void {
|
||||
if (!pending) return
|
||||
clearTimeout(pending.timer)
|
||||
pending = undefined
|
||||
}
|
||||
|
||||
function commit(): void {
|
||||
const p = pending
|
||||
if (!p) return
|
||||
dropPending()
|
||||
draftPath = p.key
|
||||
write(pendingWorkspace, p.key, p.value)
|
||||
}
|
||||
|
||||
$effect(() => {
|
||||
if (!opts.enabled() || finished) return
|
||||
const p = opts.path()
|
||||
const target = p !== '' && opts.pathError() === '' && opts.touched() ? p : ''
|
||||
if (target === untrack(() => draftPath)) return
|
||||
const t = setTimeout(() => (draftPath = target), COMMIT_DELAY_MS)
|
||||
return () => clearTimeout(t)
|
||||
const key = p !== '' && opts.pathError() === '' && opts.touched() ? p : ''
|
||||
const workspace = opts.workspace()
|
||||
const value = opts.value()
|
||||
if (key === untrack(() => draftPath)) {
|
||||
// Back on the committed key: a transition away from it is stale.
|
||||
untrack(dropPending)
|
||||
return
|
||||
}
|
||||
untrack(() => {
|
||||
dropPending()
|
||||
pendingWorkspace = workspace
|
||||
pending = { timer: setTimeout(commit, COMMIT_DELAY_MS), key, value }
|
||||
})
|
||||
})
|
||||
|
||||
$effect(() => {
|
||||
if (!opts.enabled() || finished) return
|
||||
const ws = opts.workspace()
|
||||
const p = draftPath
|
||||
const v = opts.value()
|
||||
const workspace = opts.workspace()
|
||||
const key = draftPath
|
||||
const value = opts.value()
|
||||
untrack(() => {
|
||||
if (!ws) return
|
||||
if (writtenPath && writtenPath !== p) {
|
||||
UserDraft.remove(opts.itemKind, writtenPath, { workspace: ws })
|
||||
writtenPath = ''
|
||||
}
|
||||
if (!p || v === undefined) return
|
||||
UserDraft.save(opts.itemKind, p, v, { workspace: ws })
|
||||
writtenPath = p
|
||||
if (key) write(workspace, key, value)
|
||||
})
|
||||
})
|
||||
|
||||
async function settle(): Promise<void> {
|
||||
if (!written) return
|
||||
await UserDraftDbSyncer.flush({
|
||||
workspace: written.workspace,
|
||||
itemKind: opts.itemKind,
|
||||
path: written.path
|
||||
})
|
||||
}
|
||||
|
||||
return {
|
||||
get draftPath() {
|
||||
return draftPath
|
||||
},
|
||||
finish() {
|
||||
async flush() {
|
||||
commit()
|
||||
await settle()
|
||||
},
|
||||
async finish() {
|
||||
finished = true
|
||||
const ws = untrack(() => opts.workspace())
|
||||
if (writtenPath && ws) UserDraft.remove(opts.itemKind, writtenPath, { workspace: ws })
|
||||
writtenPath = ''
|
||||
dropPending()
|
||||
const w = written
|
||||
written = undefined
|
||||
writtenValue = undefined
|
||||
draftPath = ''
|
||||
if (!w) return
|
||||
UserDraft.remove(opts.itemKind, w.path, { workspace: w.workspace })
|
||||
await UserDraftDbSyncer.flush({
|
||||
workspace: w.workspace,
|
||||
itemKind: opts.itemKind,
|
||||
path: w.path
|
||||
})
|
||||
},
|
||||
reset() {
|
||||
finished = false
|
||||
writtenPath = ''
|
||||
dropPending()
|
||||
written = undefined
|
||||
writtenValue = undefined
|
||||
draftPath = ''
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
<script lang="ts">
|
||||
import { getLocalDraftHint } from '$lib/localDraftHints.svelte'
|
||||
import { UserDraft } from '$lib/userDraft.svelte'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import { page } from '$app/state'
|
||||
import AppConnect from '$lib/components/AppConnectDrawer.svelte'
|
||||
@@ -321,14 +322,15 @@
|
||||
draftOnly?: boolean
|
||||
): Promise<void> {
|
||||
if (draftOnly) {
|
||||
// The row is the user's own draft and nothing else: the resource
|
||||
// delete would 404 on the missing deployed row.
|
||||
await UserDraftDbSyncer.save({
|
||||
// The row is the user's own draft and nothing else: the resource delete
|
||||
// would 404 on the missing deployed row. `UserDraft.remove` rather than a
|
||||
// bare POST so the in-memory cache is evicted too — the global AI chat
|
||||
// lists drafts from it and would keep offering this one.
|
||||
UserDraft.remove('resource', path, { workspace: $workspaceStore! })
|
||||
await UserDraftDbSyncer.flush({
|
||||
workspace: $workspaceStore!,
|
||||
itemKind: 'resource',
|
||||
path,
|
||||
value: null,
|
||||
immediate: true
|
||||
path
|
||||
})
|
||||
reload()
|
||||
return
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
<script lang="ts">
|
||||
import { getLocalDraftHint } from '$lib/localDraftHints.svelte'
|
||||
import { UserDraft } from '$lib/userDraft.svelte'
|
||||
import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte'
|
||||
import CenteredPage from '$lib/components/CenteredPage.svelte'
|
||||
import {
|
||||
@@ -224,14 +225,15 @@
|
||||
draftOnly?: boolean
|
||||
): Promise<void> {
|
||||
if (draftOnly) {
|
||||
// The row is the user's own draft and nothing else: the variable
|
||||
// delete would 404 on the missing deployed row.
|
||||
await UserDraftDbSyncer.save({
|
||||
// The row is the user's own draft and nothing else: the variable delete
|
||||
// would 404 on the missing deployed row. `UserDraft.remove` rather than a
|
||||
// bare POST so the in-memory cache is evicted too — the global AI chat
|
||||
// lists drafts from it and would keep offering this one.
|
||||
UserDraft.remove('variable', path, { workspace: $workspaceStore! })
|
||||
await UserDraftDbSyncer.flush({
|
||||
workspace: $workspaceStore!,
|
||||
itemKind: 'variable',
|
||||
path,
|
||||
value: null,
|
||||
immediate: true
|
||||
path
|
||||
})
|
||||
loadVariables()
|
||||
sendUserToast(`Draft ${path} was deleted`)
|
||||
|
||||
Reference in New Issue
Block a user