diff --git a/scripts/add-item-input-race.test.mjs b/scripts/add-item-input-race.test.mjs new file mode 100644 index 0000000..8a0b91b --- /dev/null +++ b/scripts/add-item-input-race.test.mjs @@ -0,0 +1,103 @@ +// Execute the actual component handlers with synchronous hook-state doubles. +// This covers the async state race, not React rendering or browser behavior. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; + +const source = readFileSync(process.env.ADD_ITEM_SOURCE ?? new URL('../src/components/AddItemInput.tsx', import.meta.url), 'utf8'); +const start = source.includes(' const submitItem = async') ? source.indexOf(' const submitItem = async') : source.indexOf(' const handleSubmit = async'); +assert.ok(start >= 0, 'component submission handlers must exist'); +const end = source.indexOf('\n return (', start); +assert.ok(end > start, 'handler boundary must exist'); +// The handler block uses just these two type annotations; do not duplicate its logic. +const handlers = source.slice(start, end).replace(/: (ItemSubmission|FormEvent)\b/g, ''); +const keys = ['name', 'isAdding', 'did', 'legacyDid', 'setName', 'setIsAdding', 'setFailedItems', 'pendingRef', 'nextSubmissionId', 'submissionsRef', 'mountedRef', 'inputRef', 'haptic', 'onAddItem', 'console']; +const compile = new Function(...keys, handlers + '\nreturn { handleSubmit, handleRetry: typeof handleRetry === "function" ? handleRetry : undefined };'); +const deferred = () => { let resolve, reject; const promise = new Promise((yes, no) => { resolve = yes; reject = no; }); return { promise, resolve, reject }; }; + +function fixture() { + const state = { name: '', isAdding: false, failedItems: [], calls: [], requests: [], feedback: [], focus: 0 }; + const env = { + did: 'did:owner', legacyDid: undefined, + setName: value => { state.name = typeof value === 'function' ? value(state.name) : value; }, + setIsAdding: value => { state.isAdding = value; }, + setFailedItems: value => { state.failedItems = typeof value === 'function' ? value(state.failedItems) : value; }, + pendingRef: { current: false }, nextSubmissionId: { current: 0 }, submissionsRef: { current: new Map() }, mountedRef: { current: true }, + inputRef: { current: { focus: () => state.focus++ } }, + haptic: value => state.feedback.push(value), console: { error() {} }, + onAddItem: args => { const request = deferred(); state.calls.push(args); state.requests.push(request); return request.promise; }, + }; + const render = () => compile(...keys.map(key => key in state ? state[key] : env[key])); + return { state, env, render, submit: () => render().handleSubmit({ preventDefault() {} }) }; +} + +test('reject A after typing B: preserve B and retain A for explicit retry', async () => { + const f = fixture(); f.state.name = ' A '; + const pending = f.submit(); + assert.equal(f.state.name, ''); + f.state.name = 'B'; f.state.requests[0].reject(new Error('queue unavailable')); await pending; + assert.equal(f.state.name, 'B'); + assert.equal(f.state.failedItems.length, 1); + assert.equal(f.state.failedItems[0].args.name, 'A'); + assert.equal(f.state.isAdding, false); +}); + +test('retry A preserves newer input and removes only A after local acceptance', async () => { + const f = fixture(); f.state.name = 'A'; const first = f.submit(); + f.state.name = 'B'; f.state.requests[0].reject(new Error('queue unavailable')); await first; + const a = f.state.failedItems[0]; + const retry = f.render().handleRetry(a); + assert.equal(f.state.name, 'B'); + assert.deepEqual(f.state.calls[1], f.state.calls[0]); + f.state.name = 'B edited'; f.state.requests[1].resolve(); await retry; + assert.equal(f.state.name, 'B edited'); assert.deepEqual(f.state.failedItems, []); + await f.render().handleRetry(a); // a stale click cannot add an accepted item again + assert.equal(f.state.calls.length, 2); +}); + +test('repeated failure keeps one retry entry; multiple failed items do not overwrite each other', async () => { + const f = fixture(); f.state.name = 'A'; const a = f.submit(); f.state.requests[0].reject(new Error('no space')); await a; + const retry = f.render().handleRetry(f.state.failedItems[0]); f.state.requests[1].reject(new Error('no space')); await retry; + assert.equal(f.state.failedItems.length, 1); + f.state.name = 'A'; const another = f.submit(); f.state.requests[2].reject(new Error('no space')); await another; + assert.equal(f.state.failedItems.length, 2); + assert.notEqual(f.state.failedItems[0].id, f.state.failedItems[1].id); + const successfulRetry = f.render().handleRetry(f.state.failedItems[0]); f.state.requests[3].resolve(); await successfulRetry; + assert.equal(f.state.failedItems.length, 1); +}); + +test('two immediate submits or retry clicks make only one local acceptance attempt', async () => { + const f = fixture(); f.state.name = 'A'; + const rendered = f.render(); const first = rendered.handleSubmit({ preventDefault() {} }); + await rendered.handleSubmit({ preventDefault() {} }); + assert.equal(f.state.calls.length, 1); + f.state.requests[0].reject(new Error('no space')); await first; + const failed = f.state.failedItems[0]; const retry = f.render(); const next = retry.handleRetry(failed); + await retry.handleRetry(failed); assert.equal(f.state.calls.length, 2); + f.state.requests[1].resolve(); await next; +}); + +for (const fails of [false, true]) test(`late ${fails ? 'failure' : 'success'} after unmount cannot change draft or feedback`, async () => { + const f = fixture(); f.state.name = 'A'; const pending = f.submit(); + f.state.name = 'New context'; f.env.mountedRef.current = false; + const before = structuredClone(f.state.feedback); + if (fails) f.state.requests[0].reject(new Error('late')); else f.state.requests[0].resolve(); + await pending; + assert.equal(f.state.name, 'New context'); assert.deepEqual(f.state.feedback, before); assert.deepEqual(f.state.failedItems, []); +}); + +test('empty input or missing identity never submits; success preserves the next draft', async () => { + const f = fixture(); f.state.name = ' '; await f.submit(); + f.state.name = 'A'; f.env.did = undefined; await f.submit(); assert.equal(f.state.calls.length, 0); + f.env.did = 'did:owner'; const pending = f.submit(); f.state.name = 'B'; f.state.requests[0].resolve(); await pending; + assert.equal(f.state.name, 'B'); assert.equal(f.state.isAdding, false); assert.deepEqual(f.state.failedItems, []); +}); + +test('source integration scopes draft by account/list and exposes explicitly ephemeral retries', () => { + assert.match(source, /key=\{JSON\.stringify\(\[did, props\.assetDid\]\)\}/); + assert.match(source, /return \(\) => \{ mountedRef\.current = false; \}/); + assert.match(source, /role="status" aria-live="polite"/); + assert.match(source, /Keep this page open to retry them/); + assert.match(source, /onClick=\{\(\) => void handleRetry\(item\)\}/); + assert.match(source, /aria-label=\{`Retry adding \$\{item\.args\.name\}`\}/); +}); diff --git a/scripts/add-item-input-ui.test.mjs b/scripts/add-item-input-ui.test.mjs new file mode 100644 index 0000000..33b52ec --- /dev/null +++ b/scripts/add-item-input-ui.test.mjs @@ -0,0 +1,99 @@ +// Render the actual component using the repository's esbuild + Happy DOM convention. +import { test, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { build } from 'esbuild'; +import { pathToFileURL } from 'node:url'; +import { GlobalRegistrator } from '@happy-dom/global-registrator'; +if (!GlobalRegistrator.isRegistered) GlobalRegistrator.register(); +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +const React = await import('react'); +const { render, cleanup, fireEvent, act } = await import('@testing-library/react'); +const fixture = globalThis.__addItemInputUi = { did: 'did:owner', feedback: [] }; + +await build({ + entryPoints: [process.env.ADD_ITEM_SOURCE ?? 'src/components/AddItemInput.tsx'], + outfile: 'tmp/add-item-input-ui.mjs', bundle: true, jsx: 'automatic', + platform: 'node', format: 'esm', external: ['react', 'react/jsx-runtime'], + plugins: [{ name: 'add-item-input-fixtures', setup(b) { + b.onResolve({ filter: /\/hooks\/(useCurrentUser|useSettings)$/ }, args => ({ path: args.path, namespace: 'fixture' })); + b.onLoad({ filter: /.*/, namespace: 'fixture' }, ({ path }) => ({ contents: path.endsWith('/useCurrentUser') + ? 'export const useCurrentUser=()=>({did:globalThis.__addItemInputUi.did});' + : 'export const useSettings=()=>({haptic:value=>globalThis.__addItemInputUi.feedback.push(value)});' })); + } }], +}); +const { AddItemInput } = await import(pathToFileURL(`${process.cwd()}/tmp/add-item-input-ui.mjs`)); +afterEach(() => { cleanup(); fixture.did = 'did:owner'; fixture.feedback = []; }); +function deferred() { + let resolve, reject; + const promise = new Promise((yes, no) => { resolve = yes; reject = no; }); + return { promise, resolve, reject }; +} +function mount() { + const calls = [], requests = []; + const ref = React.createRef(); + const props = { assetDid: 'asset:A', ref, onAddItem: args => { + calls.push(args); const request = deferred(); requests.push(request); return request.promise; + } }; + const element = () => React.createElement(React.StrictMode, null, React.createElement(AddItemInput, props)); + const view = render(element()); + const input = () => view.getByRole('textbox', { name: 'Add new item' }); + const type = value => fireEvent.change(input(), { target: { value } }); + const submit = () => fireEvent.submit(view.getByRole('form', { name: 'Add new item' })); + return { view, calls, requests, props, ref, input, type, submit, rerender: () => view.rerender(element()) }; +} + +test('rendered input keeps B when the deferred local acceptance of A rejects', async () => { + const f = mount(); f.type('A'); f.submit(); f.type('B'); + await act(async () => f.requests[0].reject(new Error('local queue unavailable'))); + assert.equal(f.input().value, 'B'); + assert.equal(f.ref.current, f.input()); + assert.ok(f.view.getByRole('button', { name: 'Retry adding A' })); + assert.match(f.view.getByRole('status').textContent, /haven't been saved on this device/); + assert.match(f.view.getByRole('status').textContent, /Keep this page open/); +}); + +test('rendered retry preserves B and its subsequent edits until A is accepted', async () => { + const f = mount(); f.type('A'); f.submit(); f.type('B'); + await act(async () => f.requests[0].reject(new Error('local queue unavailable'))); + fireEvent.click(f.view.getByRole('button', { name: 'Retry adding A' })); + assert.equal(f.input().value, 'B'); + assert.deepEqual(f.calls[1], f.calls[0]); + f.type('B edited'); + await act(async () => f.requests[1].resolve()); + assert.equal(f.input().value, 'B edited'); + assert.equal(f.view.queryByRole('button', { name: 'Retry adding A' }), null); + assert.equal(f.view.queryByRole('status'), null); +}); + +for (const scope of ['list', 'account']) { + for (const fails of [false, true]) test(`${scope} remount isolates drafts/ref and late ${fails ? 'failure' : 'success'}`, async () => { + const f = mount(); f.type('A'); f.submit(); f.type('Old draft'); + const oldInput = f.input(); + if (scope === 'list') f.props.assetDid = 'asset:B'; else fixture.did = 'did:other'; + f.rerender(); + assert.notEqual(f.input(), oldInput); + assert.equal(f.ref.current, f.input()); + assert.equal(f.input().value, ''); + f.type('C'); f.submit(); f.type('New draft'); + const feedback = [...fixture.feedback]; + await act(async () => { if (fails) f.requests[0].reject(new Error('late old failure')); else f.requests[0].resolve(); }); + assert.equal(f.input().value, 'New draft'); + assert.deepEqual(fixture.feedback, feedback); + assert.equal(f.view.queryByRole('status'), null); + // The old finally block must not unlock the new pending attempt. + f.submit(); assert.equal(f.calls.length, 2); + await act(async () => f.requests[1].resolve()); + assert.equal(f.input().value, 'New draft'); + }); +} + +test('rapid rendered submits and retry clicks queue only one acceptance at a time', async () => { + const f = mount(); f.type('A'); + act(() => { f.submit(); f.submit(); }); + assert.equal(f.calls.length, 1); + await act(async () => f.requests[0].reject(new Error('local queue unavailable'))); + const retry = f.view.getByRole('button', { name: 'Retry adding A' }); + act(() => { fireEvent.click(retry); fireEvent.click(retry); }); + assert.equal(f.calls.length, 2); + await act(async () => f.requests[1].resolve()); +}); diff --git a/src/components/AddItemInput.tsx b/src/components/AddItemInput.tsx index b15947c..ceebc31 100644 --- a/src/components/AddItemInput.tsx +++ b/src/components/AddItemInput.tsx @@ -3,7 +3,7 @@ * Features improved design, dark mode, and haptic feedback. */ -import { useState, useRef, type FormEvent, forwardRef, useImperativeHandle } from "react"; +import { useState, useRef, useEffect, type FormEvent, forwardRef, useImperativeHandle } from "react"; import { useCurrentUser } from "../hooks/useCurrentUser"; import { useSettings } from "../hooks/useSettings"; @@ -17,7 +17,18 @@ interface AddItemInputProps { }) => Promise; } -export const AddItemInput = forwardRef(function AddItemInput({ onAddItem }, ref) { +// Drafts and failed submissions must never carry into another list or account. +export const AddItemInput = forwardRef(function AddItemInput(props, ref) { + const { did } = useCurrentUser(); + return ; +}); + +type ItemSubmission = { + id: number; + args: Parameters[0]; +}; + +const AddItemInputDraft = forwardRef(function AddItemInputDraft({ onAddItem }, ref) { const { did, legacyDid } = useCurrentUser(); const { haptic } = useSettings(); const inputRef = useRef(null); @@ -28,41 +39,73 @@ export const AddItemInput = forwardRef(func const [name, setName] = useState(""); const [isAdding, setIsAdding] = useState(false); - const handleSubmit = async (e: FormEvent) => { - e.preventDefault(); - - const trimmedName = name.trim(); - if (!trimmedName || !did || isAdding) { - return; - } + const [failedItems, setFailedItems] = useState([]); + const pendingRef = useRef(false); + const nextSubmissionId = useRef(0); + const submissionsRef = useRef(new Map()); + const mountedRef = useRef(true); + useEffect(() => { + mountedRef.current = true; + return () => { mountedRef.current = false; }; + }, []); - haptic('medium'); + // onAddItem accepts the local queue entry. Subsequent server retries belong + // to OfflineRecovery; this only retains submissions rejected before that. + const submitItem = async (submission: ItemSubmission) => { + if (pendingRef.current || !mountedRef.current || submissionsRef.current.get(submission.id) !== submission) return; + pendingRef.current = true; setIsAdding(true); - setName(""); - - // Keep focus on the input immediately — don't wait for async. - // Clear the value first so the user sees it's ready for the next item. - inputRef.current?.focus(); + haptic('medium'); try { - await onAddItem({ - name: trimmedName, - createdByDid: did, - legacyDid: legacyDid ?? undefined, - createdAt: Date.now(), - }); - haptic('success'); + await onAddItem(submission.args); } catch (err) { + if (!mountedRef.current) return; console.error("Failed to add item:", err); + // Never restore an older request into the user's newer editable input. + // Keep every rejected submission separately, including identical names. + setFailedItems(items => items.some(item => item.id === submission.id) + ? items : [...items, submission]); haptic('error'); - // Restore the text on error so the user doesn't lose it - setName(trimmedName); + return; } finally { - setIsAdding(false); + pendingRef.current = false; + if (mountedRef.current) setIsAdding(false); } + if (!mountedRef.current) return; + submissionsRef.current.delete(submission.id); + setFailedItems(items => items.filter(item => item.id !== submission.id)); + // Feedback failure must not turn an accepted queue entry into a retry. + haptic('success'); + }; + + const handleSubmit = async (e: FormEvent) => { + e.preventDefault(); + const trimmedName = name.trim(); + if (!trimmedName || !did || pendingRef.current || !mountedRef.current) return; + + const submission: ItemSubmission = { + id: ++nextSubmissionId.current, + args: { + name: trimmedName, + createdByDid: did, + legacyDid: legacyDid ?? undefined, + createdAt: Date.now(), + }, + }; + submissionsRef.current.set(submission.id, submission); + setName(""); + inputRef.current?.focus(); + await submitItem(submission); + }; + + const handleRetry = async (submission: ItemSubmission) => { + // A retry never clears or replaces the next item being typed. + await submitItem(submission); }; return ( +
@@ -101,5 +144,30 @@ export const AddItemInput = forwardRef(func )} + {failedItems.length > 0 && ( +
+

+ These items haven't been saved on this device. Keep this page open to retry them. + Your current input has been kept. +

+
    + {failedItems.map(item => ( +
  • + {item.args.name} + +
    • + ))} +
+
+ )} +
); });