mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
fix(frontend): mint draft path for new SDK builder items so autosave attaches (#10056)
Fixes WIN-2159 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
7d02d9a1e4
commit
a89b896ce5
@@ -5,6 +5,7 @@
|
||||
import FlowBuilder from './FlowBuilder.svelte'
|
||||
import { usePageDraftSync } from './usePageDraftSync.svelte'
|
||||
import { workspaceStore } from '$lib/stores'
|
||||
import { selectDraftStoragePath } from '$lib/mintDraftPath'
|
||||
import type { OpenFlow } from '$lib/gen'
|
||||
|
||||
let {
|
||||
@@ -28,13 +29,19 @@
|
||||
// Stable per-user draft storage key. Captured once so editing the flow's path
|
||||
// (which lives in `draft_path`, not the storage key) can't re-key the autosave
|
||||
// handle and orphan the draft. Mirrors the full-page editor keying on the URL
|
||||
// path; falls back through the SDK's path inputs.
|
||||
const draftStoragePath = untrack(
|
||||
() =>
|
||||
props.initialPath ||
|
||||
props.pathStoreInit ||
|
||||
(oldFlowStore.val as { path?: string } | undefined)?.path ||
|
||||
''
|
||||
// path; falls back through the SDK's path inputs. For a brand-new flow with no
|
||||
// caller path this mints a `u/<user>/draft_<uuid>` key — the SDK equivalent of
|
||||
// the `/flows/add` redirect — so autosave attaches instead of the handle
|
||||
// detaching (local-only, never POSTs).
|
||||
const draftStoragePath = untrack(() =>
|
||||
selectDraftStoragePath({
|
||||
providedPaths: [
|
||||
props.initialPath,
|
||||
props.pathStoreInit,
|
||||
(oldFlowStore.val as { path?: string } | undefined)?.path
|
||||
],
|
||||
isNewItem: !!props.newFlow
|
||||
})
|
||||
)
|
||||
|
||||
// Reuse the full-page flow editor's draft orchestration so the SDK gets
|
||||
|
||||
@@ -5,12 +5,22 @@
|
||||
import type { ScriptBuilderProps } from './script_builder'
|
||||
import { usePageDraftSync } from './usePageDraftSync.svelte'
|
||||
import { workspaceStore } from '$lib/stores'
|
||||
import { selectDraftStoragePath } from '$lib/mintDraftPath'
|
||||
|
||||
let { script: oldScript, disableAi, ...props }: ScriptBuilderProps = $props()
|
||||
let { script: oldScript, disableAi, newScript, ...props }: ScriptBuilderProps = $props()
|
||||
|
||||
// Stable per-user draft storage key. Mirrors the full-page editor keying on
|
||||
// the URL path; falls back through the SDK's path inputs.
|
||||
const draftStoragePath = untrack(() => props.initialPath || oldScript?.path || '')
|
||||
// the URL path; falls back through the SDK's path inputs. For a brand-new
|
||||
// script with no caller path this mints a `u/<user>/draft_<uuid>` key — the
|
||||
// SDK equivalent of the `/scripts/add` redirect — so autosave attaches instead
|
||||
// of the handle detaching (local-only, never POSTs). Captured once (untrack)
|
||||
// so editing the path field can't re-key and orphan the draft.
|
||||
const draftStoragePath = untrack(() =>
|
||||
selectDraftStoragePath({
|
||||
providedPaths: [props.initialPath, oldScript?.path],
|
||||
isNewItem: !!newScript
|
||||
})
|
||||
)
|
||||
|
||||
// Reuse the full-page script editor's draft orchestration (same as the flow
|
||||
// SDK) so the SDK gets autosave + the AutosaveIndicator (gated by ScriptBuilder
|
||||
|
||||
@@ -15,6 +15,15 @@ export interface ScriptBuilderProps {
|
||||
disableAi?: boolean
|
||||
fullyLoaded?: boolean
|
||||
initialPath?: string
|
||||
/**
|
||||
* Wrapper-only signal (consumed by `ScriptWrapper`, not `ScriptBuilder`):
|
||||
* this editor is mounting a brand-new script. When set and no caller path
|
||||
* is provided, the wrapper mints a `u/<user>/draft_<uuid>` storage path so
|
||||
* autosave attaches — mirrors what the `/scripts/add` route does before the
|
||||
* full-page editor mounts. Left unset for read-only / pathless views so
|
||||
* autosave stays intentionally detached.
|
||||
*/
|
||||
newScript?: boolean
|
||||
/**
|
||||
* Path the route's `UserDraft.use<EditableScript>('script', ...)`
|
||||
* handle is keyed by. Distinct from `initialPath` for new drafts —
|
||||
|
||||
@@ -1,25 +1,21 @@
|
||||
import { redirect } from '@sveltejs/kit'
|
||||
import { base } from '$app/paths'
|
||||
import { getUsernameForNamespace } from '$lib/userNamespace'
|
||||
import { randomUUID } from '$lib/utils/uuid'
|
||||
import { mintDraftPath } from '$lib/mintDraftPath'
|
||||
|
||||
/**
|
||||
* Shared `load` for every `/{scripts,flows,apps,apps_raw}/add` route. Doing the
|
||||
* redirect in `load` (not `onMount`) avoids painting a blank frame first.
|
||||
*
|
||||
* Mints a fresh `u/<username>/draft_<uuid>` path and 307s to
|
||||
* Mints a fresh `u/<username>/draft_<uuid>` path (via the shared `mintDraftPath`,
|
||||
* so the SDK builder wrappers stay in lockstep) and 307s to
|
||||
* `{base}/{editPrefix}/<path>?new_draft=true&<existing-params>`. `new_draft=true`
|
||||
* tells the edit route to seed an empty editor instead of 404-ing. The hash is
|
||||
* carried over too — fork / handler-template buttons encode a payload into it
|
||||
* that the edit route's `new_draft` branch consumes. `randomUUID` not
|
||||
* `crypto.randomUUID` (WebCrypto is absent on non-secure origins).
|
||||
* that the edit route's `new_draft` branch consumes.
|
||||
*/
|
||||
export function makeDraftAddLoad(editPrefix: string) {
|
||||
return ({ url }: { url: URL }) => {
|
||||
const username = getUsernameForNamespace()
|
||||
// Underscores not dashes — path segments are `[a-zA-Z0-9_]` words and
|
||||
// downstream consumers treat `-` as foreign.
|
||||
const uuid = randomUUID().replaceAll('-', '_')
|
||||
const path = mintDraftPath()
|
||||
const params = new URLSearchParams(url.searchParams)
|
||||
params.set('new_draft', 'true')
|
||||
// `url.hash` is unavailable in `load`; read `window.location` instead
|
||||
@@ -27,6 +23,6 @@ export function makeDraftAddLoad(editPrefix: string) {
|
||||
// points at the PREVIOUS page, but every hash-payload producer arrives
|
||||
// as a full page load, so the hash is correct.
|
||||
const hash = typeof window !== 'undefined' ? window.location.hash : ''
|
||||
redirect(307, `${base}/${editPrefix}/u/${username}/draft_${uuid}?${params.toString()}${hash}`)
|
||||
redirect(307, `${base}/${editPrefix}/${path}?${params.toString()}${hash}`)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
|
||||
vi.mock('$lib/userNamespace', () => ({
|
||||
getUsernameForNamespace: () => 'alice'
|
||||
}))
|
||||
|
||||
import { mintDraftPath, selectDraftStoragePath } from './mintDraftPath'
|
||||
|
||||
const MINTED = /^u\/alice\/draft_[0-9a-f_]+$/
|
||||
|
||||
describe('mintDraftPath', () => {
|
||||
it('mints a non-empty u/<username>/draft_<uuid> path', () => {
|
||||
const path = mintDraftPath()
|
||||
expect(path).not.toBe('')
|
||||
expect(path).toMatch(MINTED)
|
||||
})
|
||||
|
||||
it('uses underscores, never dashes (path segments are word chars)', () => {
|
||||
expect(mintDraftPath()).not.toContain('-')
|
||||
})
|
||||
|
||||
it('is unique per call', () => {
|
||||
expect(mintDraftPath()).not.toBe(mintDraftPath())
|
||||
})
|
||||
})
|
||||
|
||||
describe('selectDraftStoragePath', () => {
|
||||
it('mints a non-empty path for a new item with no caller path', () => {
|
||||
// Regression: SDK create wrappers keyed autosave under '' → detached →
|
||||
// silently never POSTed. A new item must get a real, mintable key.
|
||||
const path = selectDraftStoragePath({ providedPaths: [undefined, ''], isNewItem: true })
|
||||
expect(path).toMatch(MINTED)
|
||||
})
|
||||
|
||||
it('honors the first caller-provided path over minting (SDK stopgap wins)', () => {
|
||||
expect(
|
||||
selectDraftStoragePath({ providedPaths: ['u/bob/given', undefined], isNewItem: true })
|
||||
).toBe('u/bob/given')
|
||||
})
|
||||
|
||||
it('falls through empties to the first non-empty provided path', () => {
|
||||
expect(
|
||||
selectDraftStoragePath({ providedPaths: ['', undefined, 'u/bob/existing'], isNewItem: false })
|
||||
).toBe('u/bob/existing')
|
||||
})
|
||||
|
||||
it('stays detached ("") for a non-new item with no path (read-only view)', () => {
|
||||
expect(selectDraftStoragePath({ providedPaths: [undefined, ''], isNewItem: false })).toBe('')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,42 @@
|
||||
import { getUsernameForNamespace } from '$lib/userNamespace'
|
||||
import { randomUUID } from '$lib/utils/uuid'
|
||||
|
||||
/**
|
||||
* Mint a fresh `u/<username>/draft_<uuid>` storage path for a brand-new
|
||||
* editor item. Shared by the `/{scripts,flows,apps,apps_raw}/add` route
|
||||
* redirects (`makeDraftAddLoad`) and the SDK builder wrappers
|
||||
* (`ScriptWrapper` / `FlowWrapper`) so the two can't diverge on format —
|
||||
* both need the autosave handle keyed under a real, unique path or the
|
||||
* draft detaches (local-only) and never POSTs.
|
||||
*
|
||||
* Underscores not dashes — path segments are `[a-zA-Z0-9_]` words and
|
||||
* downstream consumers treat `-` as foreign. `randomUUID` not
|
||||
* `crypto.randomUUID` (WebCrypto is absent on non-secure origins).
|
||||
*/
|
||||
export function mintDraftPath(): string {
|
||||
const username = getUsernameForNamespace()
|
||||
const uuid = randomUUID().replaceAll('-', '_')
|
||||
return `u/${username}/draft_${uuid}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the autosave storage key for an SDK builder wrapper. The first
|
||||
* caller-provided path wins (a consumer that supplies its own path — including
|
||||
* the temporary React-SDK stopgap — keeps it); otherwise, for a brand-new
|
||||
* editable item, mint a fresh draft path so autosave attaches. Returns `''`
|
||||
* (a detached, local-only handle that never POSTs) when there is no path and
|
||||
* the item isn't a new editable one — the intentionally-pathless read-only case.
|
||||
*
|
||||
* Shared by `ScriptWrapper` / `FlowWrapper` so the two can't diverge on this
|
||||
* precedence. Callers MUST capture the result once (`untrack`) so editing a
|
||||
* path field later can't re-key the handle and orphan the draft.
|
||||
*/
|
||||
export function selectDraftStoragePath(opts: {
|
||||
providedPaths: (string | undefined)[]
|
||||
isNewItem: boolean
|
||||
}): string {
|
||||
for (const p of opts.providedPaths) {
|
||||
if (p) return p
|
||||
}
|
||||
return opts.isNewItem ? mintDraftPath() : ''
|
||||
}
|
||||
@@ -746,8 +746,8 @@ function acquireEntry(
|
||||
existing.count++
|
||||
return
|
||||
}
|
||||
// Seed the cell with `defaultValue` (deep-cloned). Swallowed by
|
||||
// `skipNextWrite` below — it never POSTs.
|
||||
// Seed the cell with `defaultValue` (deep-cloned). The mirror below anchors
|
||||
// its baseline to this seed so it never POSTs the template.
|
||||
const seed = defaultValue !== undefined ? snapshotDraftValue(defaultValue) : undefined
|
||||
const cell = $state<{ val: unknown }>({ val: seed })
|
||||
const stateRef = cell as DraftState<unknown>
|
||||
@@ -780,14 +780,29 @@ function acquireEntry(
|
||||
// `cell.val` alone only subscribes to the proxy root.
|
||||
//
|
||||
// `lastSerialized` + `skipNextWrite` dedup no-op updates and treat
|
||||
// the FIRST change after mount as the seed/restore (no POST), so
|
||||
// landing on `?new_draft` doesn't sync until the user edits.
|
||||
// the seed/restore as no-POST, so landing on `?new_draft` doesn't
|
||||
// sync until the user edits.
|
||||
//
|
||||
// The swallow is anchored to the seed VALUE, not "first change after
|
||||
// mount": when a `defaultValue` seeded the cell, `lastSerialized`
|
||||
// starts at its serialization so a first mirror run that still equals
|
||||
// the seed is a genuine no-op, while a first run that already DIFFERS
|
||||
// is the user's edit and must POST. That closes a create-path race —
|
||||
// the mirror is created in a deferred microtask (see below) and the
|
||||
// handle re-keys detached→acquired as the workspace resolves, so the
|
||||
// seed and the user's first edit can land before the mirror's first
|
||||
// run and coalesce into it; a blind "swallow the first change" would
|
||||
// eat that edit.
|
||||
//
|
||||
// With no in-hand seed (page editors load & assign the value AFTER
|
||||
// mount), arm the blind first-write swallow so that post-load
|
||||
// assignment doesn't POST.
|
||||
//
|
||||
// `cell.val === undefined` is the delete signal (`value: null`).
|
||||
// `skipNextSync` lets callers that already POSTed (`discard`,
|
||||
// `remove`) suppress the duplicate fire from their own write.
|
||||
let lastSerialized: string | undefined = undefined
|
||||
let skipNextWrite = true
|
||||
let lastSerialized: string | undefined = seed === undefined ? undefined : JSON.stringify(seed)
|
||||
let skipNextWrite = seed === undefined
|
||||
$effect(() => {
|
||||
const val = cell.val
|
||||
if (val !== undefined) readFieldsRecursively(val)
|
||||
|
||||
Reference in New Issue
Block a user