Skip to content

Fix update-form reset to use a one-shot flag instead of isDirty - #349

Merged
mnindrazaka merged 1 commit into
mainfrom
claude/pos-form-reset-fix-9xo3ey
Aug 25, 2026
Merged

Fix update-form reset to use a one-shot flag instead of isDirty#349
mnindrazaka merged 1 commit into
mainfrom
claude/pos-form-reset-fix-9xo3ey

Conversation

@mnindrazaka

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #348. That PR fixed update forms silently discarding in-progress edits by switching useForm({ values: state.values }) to defaultValues plus an explicit form.reset(state.values) on fetch success, gated on !form.formState.isDirty so the reset wouldn't fire again after the user started editing.

Review feedback on #348 pointed out that isDirty is the wrong signal to gate on: it's cleared by the very form.reset() call it's supposed to guard, making it a self-referential condition whose safety depends on the timing of surrounding state transitions rather than being provably correct.

Change

Replace the isDirty check with a plain useRef flag, set once the form has been filled from the first successful fetch and never cleared again for the component's lifetime:

const hasFilledFormRef = useRef(false);
useEffect(() => {
  if (state.type === 'loaded' && !hasFilledFormRef.current) {
    form.reset(state.values);
    hasFilledFormRef.current = true;
  }
}, [state.type, state.values, form]);

This is fully decoupled from react-hook-form's dirty tracking, so the reset can only ever fire once per mount, by construction — not contingent on how or when the user happens to edit the form.

Applied across all 17 update controllers: Product, Category, Coupon, Expense, Material, RentalCheckin, RentalCheckout, StockCheck, Supplier, Table, Ticket, Transaction, Variant, Wallet, Budget, Calculation, ChecklistTemplate.

Test plan

  • nx run ui:lint — clean (only pre-existing, unrelated warnings)
  • nx run ui:test — 1158/1158 passing

https://claude.ai/code/session_019Y4KcC3TX7Z2UCBnbtBQYz


Generated by Claude Code

form.formState.isDirty is cleared by the very form.reset() call it was
gating, making it a self-referential (and therefore fragile) signal:
any code path that re-triggers the effect while the form happens to be
pristine resets it again indefinitely, with no correctness guarantee
against future changes to the surrounding state machine.

Replace it with a plain useRef flag that is set once the form has been
filled from the first successful fetch and never cleared again for the
lifetime of the component, so the reset can only ever fire once,
independent of react-hook-form's dirty tracking.
@mnindrazaka
mnindrazaka merged commit 662b1bf into main Aug 25, 2026
mnindrazaka pushed a commit that referenced this pull request Aug 26, 2026
Adds a regression test for the exact bug class fixed by PR #348/#349
(useForm({ values }) forcing the update form to re-sync from fetched
data on every render, silently discarding in-progress edits). This
locks in the current defaultValues + one-shot reset behavior for
ProductUpdateController specifically, since the fix was applied via a
broad refactor across 17 controllers and had no per-controller test
coverage.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lf13QXF7eFkx1c9KJcbnwS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants