From d8d2fdb8bba40bd72dca82b0f6273b13f450e3e3 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Sun, 18 Oct 2026 22:20:14 +0200 Subject: [PATCH 1/2] fix: keep the composer draft when the commit dialog closes Co-Authored-By: Claude Opus 5.5 --- .../js/components/commit-composer-dialog.tsx | 27 +-- resources/js/components/commit-composer.tsx | 62 ++++-- resources/js/components/issue-command.tsx | 16 +- resources/js/hooks/use-link-card.ts | 13 +- resources/js/hooks/use-stash.ts | 166 +++++++++++++-- resources/js/pages/log/show.tsx | 4 +- tests/Unit/ComposerDraft.test.tsx | 42 +++- tests/Unit/DraftStash.test.tsx | 196 +++++++++++++++++- 8 files changed, 448 insertions(+), 78 deletions(-) diff --git a/resources/js/components/commit-composer-dialog.tsx b/resources/js/components/commit-composer-dialog.tsx index 8d1fe0a7f..f3abc67fb 100644 --- a/resources/js/components/commit-composer-dialog.tsx +++ b/resources/js/components/commit-composer-dialog.tsx @@ -5,12 +5,9 @@ import { DialogDescription, DialogTitle, } from '@/components/ui/dialog'; -import { useSharedState } from '@/hooks/use-shared-state'; const bodyId = 'commit-dialog-body'; -const closedCountKey = 'commit-dialog:closed_count'; - /** * Focuses the composer at the top of the log when it is on screen, rather than opening the dialog over it. */ @@ -33,16 +30,6 @@ export function focusVisibleComposer(): boolean { return true; } -/** - * The dialog writes to the same stash as the composer on the log, so that composer remounts with this as its key to - * show the draft as the dialog left it. - */ -export function useCommitDialogClosedCount(): number { - const [closedCount] = useSharedState(closedCountKey, 0); - - return closedCount; -} - export function CommitComposerDialog({ open, onOpenChange, @@ -50,18 +37,8 @@ export function CommitComposerDialog({ open: boolean; onOpenChange: (open: boolean) => void; }) { - const [, setClosedCount] = useSharedState(closedCountKey, 0); - - const close = () => { - onOpenChange(false); - setClosedCount((count) => count + 1); - }; - return ( - (isOpen ? onOpenChange(true) : close())} - > + {open && ( { @@ -99,7 +76,7 @@ export function CommitComposerDialog({ onOpenChange(false)} /> )} diff --git a/resources/js/components/commit-composer.tsx b/resources/js/components/commit-composer.tsx index e20801eb9..729311d4f 100644 --- a/resources/js/components/commit-composer.tsx +++ b/resources/js/components/commit-composer.tsx @@ -1,7 +1,7 @@ import { Form, Link, usePage } from '@inertiajs/react'; import { ChartBar, Clock3 } from 'lucide-react'; import type { KeyboardEvent } from 'react'; -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useRef } from 'react'; import CommitController from '@/actions/App/Http/Controllers/CommitController'; import ScheduledCommitController from '@/actions/App/Http/Controllers/ScheduledCommitController'; import { ActionTooltip } from '@/components/action-tooltip'; @@ -46,6 +46,7 @@ import type { UseStashReturn } from '@/hooks/use-stash'; import { discardSeriesStash, moveCaretToEnd, + useDraftPart, useStash, } from '@/hooks/use-stash'; import { useUser } from '@/hooks/use-user'; @@ -63,10 +64,18 @@ function focusBody(id: string): void { requestAnimationFrame(() => document.getElementById(id)?.focus()); } -function useCommitSchedule() { - const [isScheduling, setIsScheduling] = useState(false); - const [scheduledAt, setScheduledAt] = useState(''); - const [timezone, setTimezone] = useState(''); +function useCommitSchedule(userId: string) { + const [isScheduling, setIsScheduling] = useDraftPart( + userId, + 'commit', + 'isScheduling', + ); + const [scheduledAt, setScheduledAt] = useDraftPart( + userId, + 'commit', + 'scheduledAt', + ); + const [timezone, setTimezone] = useDraftPart(userId, 'commit', 'timezone'); return { isScheduling, @@ -479,25 +488,21 @@ export function CommitComposer({ isInDialog?: boolean; onCommitted?: () => void; }) { - const schedule = useCommitSchedule(); - const { isScheduling, scheduledAt, setScheduledAt, setIsScheduling } = - schedule; const { user } = useUser(); + const schedule = useCommitSchedule(user.id); + const { isScheduling, scheduledAt } = schedule; const stash = useStash(user.id, 'commit'); const { body } = stash; - const linkCard = useLinkCard(body); - const [draftVersion, setDraftVersion] = useState(0); - const drop = () => { - stash.drop(); - linkCard.reset(); - setDraftVersion((version) => version + 1); - }; + const linkCard = useLinkCard( + body, + useDraftPart(user.id, 'commit', 'linkCard'), + ); const attachments = useImageAttachments( stash.images, stash.descriptions, stash.updateImages, ); - const [poll, setPoll] = useState(null); + const [poll, setPoll] = useDraftPart(user.id, 'commit', 'poll'); const secrets = useSecretWarning(body); const hasImages = attachments.images.length > 0; @@ -511,13 +516,27 @@ export function CommitComposer({ (!isScheduling || (scheduledAt !== '' && isExistingLocalTime(scheduledAt))); + const [issueKind, setIssueKind] = useDraftPart( + user.id, + 'commit', + 'issueKind', + ); + const [isOpeningIssue, setIsOpeningIssue] = useDraftPart( + user.id, + 'commit', + 'isOpeningIssue', + ); const issue = useIssueCommand({ body, updateBody: stash.updateBody, + kind: issueKind, + setKind: setIssueKind, + isOpening: isOpeningIssue, + setIsOpening: setIsOpeningIssue, enabled: !hasImages && poll === null && !isScheduling, hasSecrets: secrets.findings.length > 0, onCreated: () => { - drop(); + stash.drop(); focusBody(id); }, }); @@ -559,11 +578,8 @@ export function CommitComposer({ return; } - drop(); - setScheduledAt(''); - setIsScheduling(false); + stash.drop(); attachments.clear(); - setPoll(null); if (onCommitted) { onCommitted(); @@ -576,7 +592,7 @@ export function CommitComposer({ {({ processing, errors }) => (
)} diff --git a/resources/js/components/issue-command.tsx b/resources/js/components/issue-command.tsx index 8a21da3db..bbb58b2ab 100644 --- a/resources/js/components/issue-command.tsx +++ b/resources/js/components/issue-command.tsx @@ -30,17 +30,24 @@ function isHintDismissed(): boolean { export function useIssueCommand({ body, updateBody, + kind, + setKind, + isOpening, + setIsOpening, enabled, hasSecrets, onCreated, }: { body: string; updateBody: (body: string) => void; + kind: IssueKind | null; + setKind: (kind: IssueKind | null) => void; + isOpening: boolean; + setIsOpening: (isOpening: boolean) => void; enabled: boolean; hasSecrets: boolean; onCreated: () => void; }) { - const [kind, setKind] = useState(null); const [selected, setSelected] = useState(0); const [dismissed, setDismissed] = useState(false); const [hintDismissed, setHintDismissed] = useState(isHintDismissed); @@ -73,7 +80,7 @@ export function useIssueCommand({ }; const submit = async () => { - if (command === null || request.processing) { + if (command === null || request.processing || isOpening) { return; } @@ -85,6 +92,7 @@ export function useIssueCommand({ } request.transform(() => ({ command: command.name, body })); + setIsOpening(true); try { const issue = await request.submit(store(), { @@ -118,6 +126,8 @@ export function useIssueCommand({ }); } catch { // The callbacks explain the failure; the draft stays an issue for a retry. + } finally { + setIsOpening(false); } }; @@ -199,7 +209,7 @@ export function useIssueCommand({ } }, selected: selected % Math.max(suggestions.length, 1), - processing: request.processing, + processing: request.processing || isOpening, change, start, exit: () => setKind(null), diff --git a/resources/js/hooks/use-link-card.ts b/resources/js/hooks/use-link-card.ts index 4acef95c0..987a67444 100644 --- a/resources/js/hooks/use-link-card.ts +++ b/resources/js/hooks/use-link-card.ts @@ -11,10 +11,17 @@ export type UseLinkCardReturn = { /** * Keeps which link of a commit being written gets the link card: the first one, unless the author chose another - * link that is still in the body, or no card at all. + * link that is still in the body, or no card at all. A composer that shares its draft passes the shared choice. */ -export function useLinkCard(body: string): UseLinkCardReturn { - const [choice, setChoice] = useState(); +export function useLinkCard( + body: string, + shared?: [ + string | null | undefined, + (choice: string | null | undefined) => void, + ], +): UseLinkCardReturn { + const local = useState(); + const [choice, setChoice] = shared ?? local; const links = typedLinks(body); const chosen = choice === null || (choice !== undefined && links.includes(choice)) diff --git a/resources/js/hooks/use-stash.ts b/resources/js/hooks/use-stash.ts index 37a11b429..49daf5b79 100644 --- a/resources/js/hooks/use-stash.ts +++ b/resources/js/hooks/use-stash.ts @@ -1,16 +1,18 @@ import type { FocusEvent } from 'react'; -import { useEffect, useState, useSyncExternalStore } from 'react'; +import { useEffect, useSyncExternalStore } from 'react'; import { maxImages, pickImages, revokeObjectUrls, } from '@/components/commit-image-picker'; +import type { PollDraft } from '@/components/poll-editor'; import type { StoredImages } from '@/lib/image-storage'; import { deleteStoredImages, readStoredImages, writeStoredImages, } from '@/lib/image-storage'; +import type { IssueKind } from '@/lib/issue-commands'; import { stashableBody } from '@/lib/sensitive-text'; import { readStorage, storedKeys, writeStorage } from '@/lib/storage'; @@ -19,13 +21,47 @@ export type UseStashReturn = { readonly images: File[]; readonly descriptions: string[]; readonly isRestored: boolean; + readonly dropCount: number; readonly updateBody: (body: string) => void; readonly updateImages: (images: File[], descriptions?: string[]) => void; readonly drop: () => void; }; +/** + * The rest of a commit draft, besides its body and images. It is never stored, so it is lost on reload. + */ +export type DraftParts = { + poll: PollDraft | null; + issueKind: IssueKind | null; + isScheduling: boolean; + scheduledAt: string; + timezone: string; + linkCard: string | null | undefined; + isOpeningIssue: boolean; +}; + +const noParts: DraftParts = { + poll: null, + issueKind: null, + isScheduling: false, + scheduledAt: '', + timezone: '', + linkCard: undefined, + isOpeningIssue: false, +}; + const listeners = new Set<() => void>(); const pending = new Map(); +/** + * Every composer on a key edits the same draft, so the log composer shows what the dialog typed without remounting. + * A body with a secret lives only here, never in storage. Once no composer shows the key, it is read from storage + * again, so a draft changed in another tab is not overwritten. + */ +const drafts = new Map(); +const restored = new Map(); +const parts = new Map>(); +const drops = new Map(); +const composers = new Map(); const images = new Map(); const restoredImageKeys = new Set(); const noImages: StoredImages = { images: [], descriptions: [] }; @@ -41,8 +77,24 @@ const keyFor = (userId: string, name: string): string => const read = (key: string): string => pending.has(key) ? (pending.get(key) ?? '') : readStorage(key); +const draftFor = (key: string): string => + drafts.has(key) ? (drafts.get(key) ?? '') : read(key); + const imagesFor = (key: string): StoredImages => images.get(key) ?? noImages; +/** + * A draft found when the first composer on the key mounts is restored. The first edit fixes that answer, so text + * typed after mount never counts as restored once it is flushed to storage. + */ +const isRestoredDraft = (key: string): boolean => + restored.get(key) ?? (read(key) !== '' || imagesFor(key).images.length > 0); + +const fixRestored = (key: string, next?: boolean): void => { + if (composers.has(key)) { + restored.set(key, next ?? isRestoredDraft(key)); + } +}; + const useDraftImages = (key: string) => { const draftImages = useSyncExternalStore( subscribe, @@ -169,10 +221,21 @@ export function discardSeriesStash(userId: string): void { export function clearStashes(userId: string): void { const prefix = prefixFor(userId); - [...pending.keys(), ...storedKeys()] + [ + ...pending.keys(), + ...storedKeys(), + ...drafts.keys(), + ...restored.keys(), + ...parts.keys(), + ...drops.keys(), + ] .filter((key) => key.startsWith(prefix)) .forEach((key) => { pending.delete(key); + drafts.delete(key); + restored.delete(key); + parts.delete(key); + drops.delete(key); writeStorage(key, ''); }); @@ -203,16 +266,90 @@ export function useHasStash(userId: string | null, name: string): boolean { ); } +/** + * Every composer on the key shares the part, and dropping the draft resets it. + */ +export function useDraftPart

( + userId: string, + name: string, + part: P, +): [DraftParts[P], (next: DraftParts[P]) => void] { + const key = keyFor(userId, name); + const value = useSyncExternalStore( + subscribe, + () => { + const current = parts.get(key); + + return current !== undefined && part in current + ? (current[part] as DraftParts[P]) + : noParts[part]; + }, + () => noParts[part], + ); + + const setValue = (next: DraftParts[P]): void => { + if (!composers.has(key)) { + return; + } + + parts.set(key, { ...parts.get(key), [part]: next }); + notify(); + }; + + return [value, setValue]; +} + export function useStash(userId: string, name: string): UseStashReturn { const key = keyFor(userId, name); - const [body, setBody] = useState(() => read(key)); - const { draftImages, descriptions, isImagesRestored } = useDraftImages(key); - const [isDraftRestored, setIsDraftRestored] = useState( - () => body !== '' || draftImages.length > 0, + const body = useSyncExternalStore( + subscribe, + () => draftFor(key), + () => read(key), + ); + const setBody = (next: string): void => { + if (!composers.has(key)) { + return; + } + + fixRestored(key); + drafts.set(key, next); + notify(); + }; + const dropCount = useSyncExternalStore( + subscribe, + () => drops.get(key) ?? 0, + () => 0, + ); + const isDraftRestored = useSyncExternalStore( + subscribe, + () => isRestoredDraft(key), + () => false, ); + const { draftImages, descriptions, isImagesRestored } = useDraftImages(key); + + useEffect(() => { + composers.set(key, (composers.get(key) ?? 0) + 1); + + return () => { + const left = (composers.get(key) ?? 1) - 1; + + if (left > 0) { + composers.set(key, left); + + return; + } + + composers.delete(key); + drafts.delete(key); + restored.delete(key); + parts.delete(key); + drops.delete(key); + }; + }, [key]); useEffect(() => { if (body !== '' && stashableBody(body) === '') { + setBody(body); discardBody(key); } }, [body, key]); @@ -222,7 +359,7 @@ export function useStash(userId: string, name: string): UseStashReturn { if (next === '') { if (draftImages.length === 0) { - setIsDraftRestored(false); + fixRestored(key, false); } discardBody(key); @@ -238,16 +375,18 @@ export function useStash(userId: string, name: string): UseStashReturn { }; const updateImages = (next: File[], descriptions: string[] = []): void => { - if (next.length === 0 && body === '') { - setIsDraftRestored(false); - } - + fixRestored(key, next.length === 0 && body === '' ? false : undefined); storeImages(key, next, descriptions); }; const drop = (): void => { - setBody(''); - setIsDraftRestored(false); + if (composers.has(key)) { + drops.set(key, (drops.get(key) ?? 0) + 1); + parts.delete(key); + setBody(''); + restored.set(key, false); + } + discardBody(key); storeImages(key, []); }; @@ -259,6 +398,7 @@ export function useStash(userId: string, name: string): UseStashReturn { isRestored: (isDraftRestored || isImagesRestored) && (body !== '' || draftImages.length > 0), + dropCount, updateBody, updateImages, drop, diff --git a/resources/js/pages/log/show.tsx b/resources/js/pages/log/show.tsx index cdc34c95b..43840e998 100644 --- a/resources/js/pages/log/show.tsx +++ b/resources/js/pages/log/show.tsx @@ -4,7 +4,6 @@ import { useEffect, useRef, useState } from 'react'; import LogController from '@/actions/App/Http/Controllers/LogController'; import UserFollowController from '@/actions/App/Http/Controllers/UserFollowController'; import { CommitComposer } from '@/components/commit-composer'; -import { useCommitDialogClosedCount } from '@/components/commit-composer-dialog'; import { CommitFeed, EmptyFeed } from '@/components/commit-feed'; import { LinkifiedText } from '@/components/linkified-text'; import { PageTabs } from '@/components/page-tabs'; @@ -36,7 +35,6 @@ export default function Show({ props: { auth }, } = usePage(); const user = auth.user; - const commitDialogClosedCount = useCommitDialogClosedCount(); const viewerId = user?.id ?? 'guest'; const newestId = isRanked ? undefined : commits.data[0]?.id; const tabsRef = useRef(null); @@ -133,7 +131,7 @@ export default function Show({ {user !== null && (

- +
)} diff --git a/tests/Unit/ComposerDraft.test.tsx b/tests/Unit/ComposerDraft.test.tsx index 0a5a60073..3f0aeca72 100644 --- a/tests/Unit/ComposerDraft.test.tsx +++ b/tests/Unit/ComposerDraft.test.tsx @@ -12,13 +12,15 @@ const draft = { images: [] as File[], descriptions: [] as string[], isRestored: true, + dropCount: 0, updateBody: vi.fn(), updateImages: vi.fn((images: File[], descriptions: string[]) => { draft.images = images; draft.descriptions = descriptions; }), - drop: vi.fn(), + drop: vi.fn(() => parts.clear()), }; +const parts = new Map(); const user = { id: 'viewer', username: 'viewer', lane: 1 }; const http = { processing: false, @@ -62,11 +64,27 @@ vi.doMock('@inertiajs/react', async () => ({ vi.doMock('../../resources/js/hooks/use-user', () => ({ useUser: () => ({ user }), })); -vi.doMock('../../resources/js/hooks/use-stash', () => ({ - useStash: () => draft, - moveCaretToEnd: vi.fn(), - discardSeriesStash: vi.fn(), -})); +vi.doMock('../../resources/js/hooks/use-stash', () => { + const noParts: Record = { + poll: null, + issueKind: null, + isScheduling: false, + scheduledAt: '', + timezone: '', + linkCard: undefined, + isOpeningIssue: false, + }; + + return { + useStash: () => draft, + useDraftPart: (_userId: string, _name: string, part: string) => [ + parts.has(part) ? parts.get(part) : noParts[part], + (value: unknown) => parts.set(part, value), + ], + moveCaretToEnd: vi.fn(), + discardSeriesStash: vi.fn(), + }; +}); const { CommitComposer } = await import('../../resources/js/components/commit-composer'); @@ -119,6 +137,7 @@ beforeEach(() => { draft.body = 'A restored draft https://example.com'; draft.images = []; draft.descriptions = []; + parts.clear(); }); test('preserves restored images and link cards and allows scheduling them', () => { @@ -506,6 +525,16 @@ test('picking a suggestion with Enter starts an empty issue, and the shortcut op command: '/bug', body: 'Search loses its filters', }); + + state = []; + const otherComposer = render(); + keyDown(otherComposer, 'Enter', { metaKey: true }); + assert.equal(http.submit.mock.calls.length, 1); + assert.equal( + otherComposer.fields.find((element) => element.type === 'fieldset') + ?.props.disabled, + true, + ); }); test('a draft that mentions a bug suggests /bug, keeps the text when used, and can be dismissed', () => { @@ -534,6 +563,7 @@ test('a draft that mentions a bug suggests /bug, keeps the text when used, and c ); state = []; + parts.clear(); hint = render().fields.find( (element) => element.type === IssueCommandHint, )!; diff --git a/tests/Unit/DraftStash.test.tsx b/tests/Unit/DraftStash.test.tsx index ea9d74603..1b0538af2 100644 --- a/tests/Unit/DraftStash.test.tsx +++ b/tests/Unit/DraftStash.test.tsx @@ -25,10 +25,17 @@ const storedImages = new Map< let state: unknown[] = []; let cursor = 0; +let cleanups: (() => void)[] = []; vi.doMock('react', () => ({ ...React, - useEffect: (effect: () => void) => effect(), + useEffect: (effect: () => (() => void) | void) => { + const cleanup = effect(); + + if (cleanup) { + cleanups.push(cleanup); + } + }, useState: (initial: unknown) => { const index = cursor++; @@ -85,7 +92,7 @@ vi.doMock('../../resources/js/lib/image-storage', () => ({ }, })); -const { clearStashes, useHasStash, useStash } = +const { clearStashes, useDraftPart, useHasStash, useStash } = await import('../../resources/js/hooks/use-stash'); test('restores draft images, preserves them through body edits, and drops the complete draft', async () => { @@ -328,3 +335,188 @@ test('drafts release image URLs on replacement, drop and logout while keeping re revoke.mockRestore(); } }); + +test('the log composer and the dialog share one live draft, so closing the dialog needs no remount', () => { + const token = 'ghp_' + 'Ab3dEf6hIj9kLm2nOp5qRs8tUv1wXy4zABCD'; + const key = 'stash:author:shared'; + const composer = () => { + let own: unknown[] = []; + + return () => { + state = own; + cursor = 0; + const draft = useStash('author', 'shared'); + own = state; + + return draft; + }; + }; + const log = composer(); + const dialog = composer(); + vi.stubGlobal('window', { addEventListener() {} }); + + try { + log().updateBody(token); + assert.equal(dialog().body, token); + assert.equal(bodies.has(key), false); + + dialog().updateBody('Typed in the dialog'); + assert.equal(log().body, 'Typed in the dialog'); + + assert.equal(log().dropCount, 0); + dialog().drop(); + assert.equal(log().body, ''); + assert.equal(log().dropCount, 1); + + clearStashes('author'); + assert.equal(log().body, ''); + assert.equal(dialog().body, ''); + } finally { + clearStashes('author'); + vi.unstubAllGlobals(); + } +}); + +test('a draft is read from storage again once no composer shows it, so another tab can change it', () => { + const key = 'stash:author:tabs'; + bodies.set(key, 'Written here'); + state = []; + cleanups = []; + cursor = 0; + + try { + assert.equal(useStash('author', 'tabs').body, 'Written here'); + cleanups.forEach((cleanup) => cleanup()); + bodies.set(key, 'Changed in another tab'); + + state = []; + cursor = 0; + assert.equal(useStash('author', 'tabs').body, 'Changed in another tab'); + } finally { + clearStashes('author'); + } +}); + +test('the log composer and the dialog share the rest of the draft, and a drop resets it for both', () => { + const poll = { options: ['Tabs', 'Spaces'], duration: 60 }; + state = []; + cursor = 0; + vi.stubGlobal('window', { addEventListener() {} }); + + try { + useStash('author', 'commit'); + useDraftPart('author', 'commit', 'poll')[1](poll); + useDraftPart('author', 'commit', 'issueKind')[1]('bug'); + useDraftPart('author', 'commit', 'isScheduling')[1](true); + useDraftPart('author', 'commit', 'linkCard')[1](null); + + assert.deepEqual(useDraftPart('author', 'commit', 'poll')[0], poll); + assert.equal(useDraftPart('author', 'commit', 'issueKind')[0], 'bug'); + assert.equal(useDraftPart('author', 'commit', 'isScheduling')[0], true); + assert.equal(useDraftPart('author', 'commit', 'linkCard')[0], null); + assert.equal(useDraftPart('another', 'commit', 'poll')[0], null); + + useStash('author', 'commit').drop(); + + assert.equal(useDraftPart('author', 'commit', 'poll')[0], null); + assert.equal(useDraftPart('author', 'commit', 'issueKind')[0], null); + assert.equal(useDraftPart('author', 'commit', 'isScheduling')[0], false); + assert.equal(useDraftPart('author', 'commit', 'linkCard')[0], undefined); + + useDraftPart('author', 'commit', 'poll')[1](poll); + clearStashes('author'); + assert.equal(useDraftPart('author', 'commit', 'poll')[0], null); + } finally { + clearStashes('author'); + vi.unstubAllGlobals(); + } +}); + +test('the log composer and the dialog agree on whether the draft is restored', () => { + const key = 'stash:author:restored'; + const composer = () => { + let own: unknown[] = []; + + return () => { + state = own; + cursor = 0; + const draft = useStash('author', 'restored'); + own = state; + + return draft; + }; + }; + const log = composer(); + const dialog = composer(); + bodies.set(key, 'Saved before reload'); + vi.stubGlobal('window', { addEventListener() {} }); + + try { + assert.equal(log().isRestored, true); + assert.equal(dialog().isRestored, true); + + dialog().updateBody(''); + assert.equal(log().isRestored, false); + + log().updateBody('Typed after the page loaded'); + assert.equal(log().isRestored, false); + assert.equal(dialog().isRestored, false); + } finally { + clearStashes('author'); + vi.unstubAllGlobals(); + } +}); + +test('a composer that finishes after it unmounts leaves nothing in memory, so storage is read again', () => { + const key = 'stash:author:late'; + bodies.set(key, 'Written here'); + state = []; + cleanups = []; + cursor = 0; + vi.stubGlobal('window', { addEventListener() {} }); + + try { + const draft = useStash('author', 'late'); + const [, setKind] = useDraftPart('author', 'late', 'issueKind'); + cleanups.forEach((cleanup) => cleanup()); + + setKind('bug'); + draft.drop(); + bodies.set(key, 'Changed in another tab'); + + state = []; + cursor = 0; + const remounted = useStash('author', 'late'); + assert.equal(remounted.body, 'Changed in another tab'); + assert.equal(remounted.dropCount, 0); + assert.equal(useDraftPart('author', 'late', 'issueKind')[0], null); + } finally { + clearStashes('author'); + vi.unstubAllGlobals(); + } +}); + +test('a stored draft with a secret stays in the editor while it leaves storage', () => { + const token = 'ghp_' + 'Ab3dEf6hIj9kLm2nOp5qRs8tUv1wXy4zABCD'; + const key = 'stash:author:stored-secret'; + bodies.set(key, token); + state = []; + cleanups = []; + cursor = 0; + vi.stubGlobal('window', { addEventListener() {} }); + + try { + useStash('author', 'stored-secret'); + assert.equal(bodies.has(key), false); + + state = []; + cursor = 0; + const draft = useStash('author', 'stored-secret'); + assert.equal(draft.body, token); + assert.equal(draft.isRestored, true); + } finally { + cleanups.forEach((cleanup) => cleanup()); + clearStashes('author'); + vi.unstubAllGlobals(); + } +}); From f3353c96002aca6fe1db0563b0adbbd11520daf3 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Sun, 18 Oct 2026 22:36:47 +0200 Subject: [PATCH 2/2] fix: keep the stash effect free of hook-local functions Co-Authored-By: Claude Opus 5.5 --- resources/js/hooks/use-stash.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/resources/js/hooks/use-stash.ts b/resources/js/hooks/use-stash.ts index 49daf5b79..abc44d4d2 100644 --- a/resources/js/hooks/use-stash.ts +++ b/resources/js/hooks/use-stash.ts @@ -95,6 +95,16 @@ const fixRestored = (key: string, next?: boolean): void => { } }; +const writeDraft = (key: string, next: string): void => { + if (!composers.has(key)) { + return; + } + + fixRestored(key); + drafts.set(key, next); + notify(); +}; + const useDraftImages = (key: string) => { const draftImages = useSyncExternalStore( subscribe, @@ -306,15 +316,7 @@ export function useStash(userId: string, name: string): UseStashReturn { () => draftFor(key), () => read(key), ); - const setBody = (next: string): void => { - if (!composers.has(key)) { - return; - } - - fixRestored(key); - drafts.set(key, next); - notify(); - }; + const setBody = (next: string): void => writeDraft(key, next); const dropCount = useSyncExternalStore( subscribe, () => drops.get(key) ?? 0, @@ -349,7 +351,7 @@ export function useStash(userId: string, name: string): UseStashReturn { useEffect(() => { if (body !== '' && stashableBody(body) === '') { - setBody(body); + writeDraft(key, body); discardBody(key); } }, [body, key]);