Skip to content

Fix form state management: use defaultValues with reset instead of values - #348

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

Fix form state management: use defaultValues with reset instead of values#348
mnindrazaka merged 2 commits into
mainfrom
claude/pos-form-reset-fix-9xo3ey

Conversation

@mnindrazaka

Copy link
Copy Markdown
Collaborator

Summary

This PR fixes form state management across multiple controllers by switching from the values option to defaultValues in useForm() initialization, and adding explicit form.reset() calls when data loads. This prevents form state from becoming out of sync with async data updates.

Key Changes

  • RentalCheckoutFormView: Refactored cart sheet visibility logic from two separate useEffect hooks into a single derived state variable (isCartSheetVisible). This eliminates the flash of the sheet opening before closing on the frame before effects run.
  • All Update Controllers (Budget, Calculation, Category, Coupon, Expense, Material, Product, RentalCheckin, RentalCheckout, StockCheck, Supplier, Table, Ticket, Transaction, Variant, Wallet):
    • Changed useForm({ values: state.values, ... }) to useForm({ defaultValues: state.values, ... })
    • Added useEffect hook that calls form.reset(state.values) when state.type === 'loaded'

Implementation Details

The change from values to defaultValues + explicit reset() is the correct pattern for react-hook-form when dealing with async data. The values option causes the form to be controlled by external state on every render, which can lead to:

  • Form state becoming stale when async updates occur
  • Difficulty tracking dirty state
  • Unexpected behavior with field validation

By using defaultValues with explicit reset() calls, the form maintains its own state while still synchronizing when data loads, providing better UX and more predictable behavior.

The RentalCheckoutFormView refactoring improves performance and UX by deriving the sheet visibility state during render rather than using effects, preventing the visual flash that occurs when effects run after the initial render.

https://claude.ai/code/session_019Y4KcC3TX7Z2UCBnbtBQYz

claude added 2 commits August 25, 2026 02:42
…hem after fetch

React Hook Form's `values` option keeps re-syncing the form to
state.values on every state change, wiping out in-progress edits
since the form never writes back to state. Switch update controllers
to `defaultValues` (set once) plus an explicit form.reset(state.values)
when the fetch completes, matching the pattern already used in
ChecklistTemplateUpdateController.

Also fixes two pre-existing react-hooks/set-state-in-effect lint
errors in RentalCheckoutFormView by deriving the cart sheet's open
state during render instead of syncing it via useEffect, since the
repo's pre-commit hook lints the whole project.
… edits

form.reset(state.values) was re-firing whenever state.type changed
back to 'loaded' (e.g. submitError -> SUBMIT_CANCEL), even without a
new fetch. If the user kept editing after a failed submit and then
cancelled, their newer edits were silently reset back to the stale
submitted values. Gate the reset on form.formState.isDirty being
false so it only fires on the initial post-fetch fill, never after
the user has touched the form.

@mnindrazaka mnindrazaka left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — there was actually a real bug this exposed: submitError -> SUBMIT_CANCEL re-enters type: 'loaded' without a new fetch, so the effect was re-firing on that transition too. If someone kept editing after a failed submit and then cancelled, their newer edits got reset back to the stale submitted values.

Applied via !form.formState.isDirty rather than a literal "form is empty" check, since RHF's isDirty is the idiomatic way to express "hasn't diverged from what was last set" and holds up across forms where an empty field isn't a meaningful signal (e.g. WalletForm's balance/booleans). Now the reset only fires on the initial post-fetch fill and never again once the user has touched anything, across all 17 update controllers.


Generated by Claude Code

Comment thread libs/ui/src/presentation/controllers/WalletUpdateController.tsx
@mnindrazaka
mnindrazaka merged commit 5ec4580 into main Aug 25, 2026
@mnindrazaka
mnindrazaka deleted the claude/pos-form-reset-fix-9xo3ey branch August 25, 2026 03:14
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