fix(ui): capture event values before deferring to mutate()
The "Income this month" field could not be changed from 0. Typing zeros appeared to work; typing any non-zero snapped the field back to a single 0. mutate() defers its callback behind the store's serialization queue and an IndexedDB write. React re-renders and resets a controlled input's DOM value back to the last committed value well before that callback runs, so a handler reading event.target.value INSIDE the callback read the reset value and committed 0 — forever. Committing 0 matches what is displayed, which is why typing zeros looked fine. Six handlers had this shape (income, post name, post currency, standing-rule kind, base currency, rate service URL). Each now reads the DOM value synchronously and passes it in. Sites that already captured into a const first — the FX rate and the standing-rule amount — were never affected, which is why those fields worked. Purchase, split and plan editors were never affected either: they write to local component state synchronously, never through the deferred queue. Adds a source-level regression guard, because the trigger is real-browser render timing that happy-dom does not reproduce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d1d6735596
commit
417adb0d85
@@ -0,0 +1,43 @@
|
||||
import { test, expect } from "bun:test";
|
||||
import { Glob } from "bun";
|
||||
|
||||
/**
|
||||
* Regression guard for a bug that froze the "Income this month" field at 0.
|
||||
*
|
||||
* `mutate()` defers its callback behind the store's serialization queue and an
|
||||
* IndexedDB write. React re-renders and resets a controlled input's DOM value
|
||||
* back to the last committed value long before that callback runs, so a handler
|
||||
* that reads `event.target.value` INSIDE the callback reads the reset value —
|
||||
* committing 0 forever, while typing appeared to do nothing.
|
||||
*
|
||||
* The value must be captured synchronously in the handler, then passed in.
|
||||
* This scans source rather than behaviour because the trigger is real-browser
|
||||
* render timing that happy-dom does not reproduce.
|
||||
*/
|
||||
test("no handler reads event.target.value inside a deferred mutate() callback", async () => {
|
||||
const offenders: string[] = [];
|
||||
|
||||
for await (const file of new Glob("src/ui/**/*.tsx").scan(".")) {
|
||||
const source = await Bun.file(file).text();
|
||||
// Find each mutate( call and inspect its callback body.
|
||||
const re = /mutate\(\s*\(?\s*(?:draft|data)\b/g;
|
||||
let match: RegExpExecArray | null;
|
||||
while ((match = re.exec(source)) !== null) {
|
||||
// Walk to the end of the mutate(...) call by matching parentheses.
|
||||
let depth = 0;
|
||||
let i = source.indexOf("(", match.index);
|
||||
const start = i;
|
||||
for (; i < source.length; i++) {
|
||||
if (source[i] === "(") depth++;
|
||||
else if (source[i] === ")") { depth--; if (depth === 0) break; }
|
||||
}
|
||||
const body = source.slice(start, i + 1);
|
||||
if (/\b(?:event|e)\.target\.value/.test(body)) {
|
||||
const line = source.slice(0, match.index).split("\n").length;
|
||||
offenders.push(`${file}:${line}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
@@ -49,14 +49,12 @@ export function MonthRoute() {
|
||||
step="0.01"
|
||||
className="font-money w-40"
|
||||
value={view.income}
|
||||
onChange={(event) =>
|
||||
mutate((draft) =>
|
||||
setIncome(draft, monthId, {
|
||||
amount: Number(event.target.value) || 0,
|
||||
currency: base,
|
||||
}),
|
||||
)
|
||||
}
|
||||
onChange={(event) => {
|
||||
// Read the DOM value NOW: mutate() defers behind the write queue,
|
||||
// by which time React has reset this input to the committed value.
|
||||
const amount = Number(event.target.value) || 0;
|
||||
mutate((draft) => setIncome(draft, monthId, { amount, currency: base }));
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
<dl className="flex gap-6 text-sm">
|
||||
|
||||
@@ -44,9 +44,10 @@ export function CurrencySection() {
|
||||
id="base-currency"
|
||||
className="h-9 w-32 rounded border bg-background px-2 text-sm"
|
||||
value={base}
|
||||
onChange={(event) =>
|
||||
mutate((draft) => setBaseCurrency(draft, event.target.value as Currency))
|
||||
}
|
||||
onChange={(event) => {
|
||||
const currency = event.target.value as Currency;
|
||||
mutate((draft) => setBaseCurrency(draft, currency));
|
||||
}}
|
||||
>
|
||||
{CURRENCIES.map((currency) => (
|
||||
<option key={currency} value={currency}>
|
||||
@@ -115,11 +116,12 @@ export function CurrencySection() {
|
||||
id="fx-url"
|
||||
className="max-w-xl"
|
||||
value={apiUrl}
|
||||
onChange={(event) =>
|
||||
onChange={(event) => {
|
||||
const url = event.target.value;
|
||||
mutate((draft) => {
|
||||
draft.settings.fxApiUrl = event.target.value;
|
||||
})
|
||||
}
|
||||
draft.settings.fxApiUrl = url;
|
||||
});
|
||||
}}
|
||||
/>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{"{base}"} and {"{targets}"} are substituted. Fetching is optional — the app
|
||||
|
||||
@@ -19,16 +19,17 @@ export function PostsSection() {
|
||||
<select
|
||||
className="h-8 rounded border bg-background px-1 text-xs"
|
||||
value={rule.kind}
|
||||
onChange={(event) =>
|
||||
onChange={(event) => {
|
||||
const kind = event.target.value;
|
||||
mutate((draft) =>
|
||||
updatePost(draft, postId, {
|
||||
standingRule:
|
||||
event.target.value === "fixed"
|
||||
kind === "fixed"
|
||||
? { kind: "fixed", amount: { amount: 0, currency: base } }
|
||||
: { kind: "percentOfIncome", percent: 0 },
|
||||
}),
|
||||
)
|
||||
}
|
||||
);
|
||||
}}
|
||||
>
|
||||
<option value="fixed">fixed amount</option>
|
||||
<option value="percentOfIncome">% of income</option>
|
||||
@@ -82,20 +83,20 @@ export function PostsSection() {
|
||||
<Input
|
||||
className="h-8 w-48"
|
||||
value={post.name}
|
||||
onChange={(event) =>
|
||||
mutate((draft) => updatePost(draft, post.id, { name: event.target.value }))
|
||||
}
|
||||
onChange={(event) => {
|
||||
const name = event.target.value;
|
||||
mutate((draft) => updatePost(draft, post.id, { name }));
|
||||
}}
|
||||
/>
|
||||
</td>
|
||||
<td className="py-2">
|
||||
<select
|
||||
className="h-8 rounded border bg-background px-1 text-xs"
|
||||
value={post.currency}
|
||||
onChange={(event) =>
|
||||
mutate((draft) =>
|
||||
updatePost(draft, post.id, { currency: event.target.value as Currency }),
|
||||
)
|
||||
}
|
||||
onChange={(event) => {
|
||||
const currency = event.target.value as Currency;
|
||||
mutate((draft) => updatePost(draft, post.id, { currency }));
|
||||
}}
|
||||
>
|
||||
{CURRENCIES.map((currency) => (
|
||||
<option key={currency} value={currency}>
|
||||
|
||||
Reference in New Issue
Block a user