Fix controlled input transform handling#9
Conversation
|
Warning Review limit reached
More reviews will be available in 45 minutes and 37 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6550cf4d77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onChange(evt); | ||
| } | ||
| field.onChange(evt); | ||
| field.onChange(hasTransform ? transformValue(getEventValue(evt), rules) : evt); |
There was a problem hiding this comment.
Keep transformed values out of the DOM input value
When a transform rule is enabled, this stores the transformed value directly in RHF state, but the next render still spreads field.value back into <Input> as the DOM value. That breaks common transform cases: clearing a valueAsNumber field stores NaN and produces a React input warning, while valueAsDate stores a Date object that a type="date" input cannot display as its required yyyy-mm-dd string. The component needs a separate serialized display value (or equivalent mapping) while keeping the transformed value in form state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🍛 Currybot Review
📋 Context
- Ticket: N/A — PR is scoped to fixing
ControlledInputsupport for RHF transform helpers (valueAsNumber,valueAsDate,setValueAs) and surfacingControlledCurrencyInputerrors. - Related: RAG searches for this PR/topic returned no matching prior docs or decisions.
- Cursor rules consulted:
medusa-forms-patterns.mdc,medusa-stories-patterns.mdc,react-typescript-patterns.mdc,ui-component-patterns.mdc,storybook-testing.mdc. - DevAgent plan: N/A.
- Scope: ✅ matches the PR description.
🔍 Gap Analysis
- The implementation addresses the core gap: transform-only RHF rule options are now accepted by
ControlledInput, applied before updating RHF state, and intentionally stripped before passing validation rules toController. ControlledCurrencyInputnow passesformErrorsthrough to the UI component, which aligns with the Medusa FormsFieldWrappererror-display pattern.- No ticket or plan context was available beyond the PR body, so there were no external acceptance criteria to validate against.
🧪 Code Quality
- ✅
valueAsNumber,valueAsDate, andsetValueAshandling is small and localized. - ✅ Existing event-based behavior is preserved when no transform rule is present.
- ✅ The controlled component still calls consumer
onChangebefore updating RHF state, matching the established controlled component pattern. ⚠️ The new Storybook examples demonstrate the transform behavior visually, but they do not add interaction/play assertions. Since this is regression-prone form behavior, adding Storybookplaychecks forValueAsNumberandSetValueAswould make the coverage stronger.
🔧 Simplicity Audit
- No violations found.
- Simplicity deductions applied: 0. The helper extraction is proportionate to the problem and avoids pushing transform-specific logic into the render callback.
🌟 Highlights
- Nice preservation of the default
field.onChange(evt)path; that keeps behavior stable for regular text inputs. - The rule destructuring makes the intent clear: RHF validation rules still go through
Controller, while transform helpers are handled locally. - Good docs coverage via focused stories that show the resulting value/type, which makes the fix easy to inspect manually.
📊 Merge Confidence: 9.2/10
- ✅ Builds and typechecks passed locally:
./node_modules/.bin/tsc --noEmit -p packages/medusa-forms/tsconfig.json --types vite/client./node_modules/.bin/biome check ...touched files...node .yarn/releases/yarn-4.9.1.cjs workspace @lambdacurry/medusa-forms buildnode .yarn/releases/yarn-4.9.1.cjs workspace @lambdacurry/medusa-forms-docs build-storybook
⚠️ Would raise to 9.5+ with executable Storybookplayassertions for the two transform stories.- 🔧 Simplicity deductions applied: 0 total.
6550cf4 to
77c5416
Compare
77c5416 to
c16560f
Compare
c16560f to
8b3a21a
Compare
8b3a21a to
b7e35e2
Compare
b7e35e2 to
f171e01
Compare
Summary
Validation
Notes