feat(conform-react): preserve intent value types - #1289
Conversation
🦋 Changeset detectedLatest commit: ac37616 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying conform with
|
| Latest commit: |
ac37616
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2ac9e31a.conform.pages.dev |
| Branch Preview URL: | https://edmundhung-preserve-intent-v.conform.pages.dev |
📝 WalkthroughWalkthroughThis change refines ChangesIntent inference
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
More templates
@conform-to/dom
@conform-to/react
@conform-to/valibot
@conform-to/validitystate
@conform-to/yup
@conform-to/zod
commit: |
3ab3727 to
861bb6a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/conform-react/future/types.ts`:
- Around line 696-700: Update IntentDispatch to accept a ResultType generic and
pass it to ExtractDispatchSignature while keeping serialize fixed to string;
update IntentDispatcher’s IntentHandler mapping to infer and forward its second
generic argument so programmatic dispatch preserves configured non-void results.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fa07d0f0-f84f-4fb2-af07-773754048e5e
📒 Files selected for processing (3)
.changeset/fair-intents-infer.mdpackages/conform-react/future/types.tspackages/conform-react/tests/types.test-d.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (11)
- GitHub Check: Validate Preview / Epic Stack
- GitHub Check: Validate Preview / Examples
- GitHub Check: Future API Tests (ubuntu-latest, webkit, 22)
- GitHub Check: E2E Tests (ubuntu-latest, firefox, 22)
- GitHub Check: Future API Tests (macos-latest, chromium, 22)
- GitHub Check: E2E Tests (ubuntu-latest, webkit, 22)
- GitHub Check: Future API Tests (macos-latest, webkit, 22)
- GitHub Check: E2E Tests (macos-latest, chromium, 22)
- GitHub Check: E2E Tests (windows-latest, chromium, 22)
- GitHub Check: Future API Tests (windows-latest, chromium, 22)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-04-05T20:22:11.135Z
Learnt from: edmundhung
Repo: edmundhung/conform PR: 1180
File: docs/api/react/future/useControl.md:361-363
Timestamp: 2026-04-05T20:22:11.135Z
Learning: When reviewing usages of `useControl.register` in `packages/conform-react`, consider both supported registration forms valid: (1) passing a single form control element (`HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement | HTMLFieldSetElement`), and (2) passing an iterable collection (`NodeList`/`HTMLCollection`) of checkbox/radio inputs that share the same `name` and compatible `type`. Do not flag the collection-based group registration as an error—both patterns are valid and maintained.
Applied to files:
packages/conform-react/tests/types.test-d.tspackages/conform-react/future/types.ts
📚 Learning: 2026-07-18T14:14:07.849Z
Learnt from: edmundhung
Repo: edmundhung/conform PR: 1244
File: packages/conform-react/future/intent.ts:70-75
Timestamp: 2026-07-18T14:14:07.849Z
Learning: For the React “future” custom-state API, the `customState` handler map must be captured on form mount and remain unchanged when the form is reset. When reviewing changes to the future `useForm`/custom-state implementation, ensure reset logic does not recreate or mutate the captured handler map (the handler map should stay referentially stable per the contract described in `docs/api/react/future/useForm.md`).
Applied to files:
packages/conform-react/future/types.ts
🪛 markdownlint-cli2 (0.23.1)
.changeset/fair-intents-infer.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (4)
packages/conform-react/future/types.ts (2)
713-717: LGTM!Also applies to: 725-746
748-761: 🎯 Functional CorrectnessVerify the literal
name: undefinedpath.
OptionalNameUpdateIntentOptions<undefined>permitsindex?: numberandvalue: unknown. If inference selects this branch for a literalname: undefined, it bypasses the typed whole-form update variant and permits an index that the whole-form variant rejects.Add direct and serialized regression cases for an invalid root value without
index, and for a valid root value withindex: 0. If either call compiles, exclude an exactlyundefinedNamefrom this branch while retaining support for name types that also containstring.packages/conform-react/tests/types.test-d.ts (1)
171-288: LGTM!.changeset/fair-intents-infer.md (1)
1-5: LGTM!
861bb6a to
cf1347a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/conform-react/future/types.ts (1)
696-700:⚠️ Potential issue | 🟠 MajorForward the configured
IntentHandlerresult type.
IntentDispatchomits a result type and usesExtractDispatchSignaturewith itsvoiddefault.IntentDispatcherat Line 807 matches the secondIntentHandlergeneric asanyand does not forward it. A non-void handler therefore has avoidprogrammatic dispatch signature.Add
ResultTypetoIntentDispatch, then infer and forward it fromIntentHandler. Keepserializefixed tostring. Add a compile-time assertion for a non-void handler result.Proposed fix
export type IntentDispatch< Intent extends IntentDefinition, FormShape extends Record<string, any> = Record<string, any>, -> = ExtractDispatchSignature<NormalizeIntentType<Intent>, FormShape> & { + ResultType = void, +> = ExtractDispatchSignature< + NormalizeIntentType<Intent>, + FormShape, + ResultType +> & { serialize: ExtractDispatchSignature< NormalizeIntentType<Intent>, FormShape, string >; }; - ? IntentDispatch<Dispatch, FormShape> + ? IntentDispatch<Dispatch, FormShape, ResultType>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/conform-react/future/types.ts` around lines 696 - 700, Update IntentDispatch and the IntentDispatcher flow to carry the configured IntentHandler result type: add a ResultType generic to IntentDispatch, infer it from IntentHandler, and pass it to ExtractDispatchSignature while keeping serialize fixed as string. Add a compile-time assertion covering a non-void handler result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/conform-react/future/types.ts`:
- Around line 696-700: Update IntentDispatch and the IntentDispatcher flow to
carry the configured IntentHandler result type: add a ResultType generic to
IntentDispatch, infer it from IntentHandler, and pass it to
ExtractDispatchSignature while keeping serialize fixed as string. Add a
compile-time assertion covering a non-void handler result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a175380-b369-49a7-83bd-1bba33289de4
📒 Files selected for processing (4)
.changeset/fair-intents-infer.mdpackages/conform-react/future/types.tspackages/conform-react/tests/types.test-d.tspackages/conform-react/tests/useControl.browser.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: Validate Preview / Examples
- GitHub Check: Validate Preview / Epic Stack
- GitHub Check: Future API Tests (macos-latest, webkit, 22)
- GitHub Check: Future API Tests (macos-latest, chromium, 22)
- GitHub Check: Future API Tests (windows-latest, chromium, 22)
- GitHub Check: Future API Tests (ubuntu-latest, chromium, 22)
- GitHub Check: E2E Tests (macos-latest, chromium, 22)
- GitHub Check: Future API Tests (ubuntu-latest, webkit, 22)
- GitHub Check: Future API Tests (ubuntu-latest, firefox, 22)
- GitHub Check: E2E Tests (macos-latest, webkit, 22)
- GitHub Check: E2E Tests (ubuntu-latest, webkit, 22)
- GitHub Check: E2E Tests (windows-latest, chromium, 22)
- GitHub Check: E2E Tests (ubuntu-latest, firefox, 22)
- GitHub Check: E2E Tests (ubuntu-latest, chromium, 22)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-04-05T20:22:11.135Z
Learnt from: edmundhung
Repo: edmundhung/conform PR: 1180
File: docs/api/react/future/useControl.md:361-363
Timestamp: 2026-04-05T20:22:11.135Z
Learning: When reviewing usages of `useControl.register` in `packages/conform-react`, consider both supported registration forms valid: (1) passing a single form control element (`HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement | HTMLFieldSetElement`), and (2) passing an iterable collection (`NodeList`/`HTMLCollection`) of checkbox/radio inputs that share the same `name` and compatible `type`. Do not flag the collection-based group registration as an error—both patterns are valid and maintained.
Applied to files:
packages/conform-react/tests/useControl.browser.test.tsxpackages/conform-react/tests/types.test-d.tspackages/conform-react/future/types.ts
📚 Learning: 2026-07-18T14:14:07.849Z
Learnt from: edmundhung
Repo: edmundhung/conform PR: 1244
File: packages/conform-react/future/intent.ts:70-75
Timestamp: 2026-07-18T14:14:07.849Z
Learning: For the React “future” custom-state API, the `customState` handler map must be captured on form mount and remain unchanged when the form is reset. When reviewing changes to the future `useForm`/custom-state implementation, ensure reset logic does not recreate or mutate the captured handler map (the handler map should stay referentially stable per the contract described in `docs/api/react/future/useForm.md`).
Applied to files:
packages/conform-react/future/types.ts
🪛 markdownlint-cli2 (0.23.1)
.changeset/fair-intents-infer.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (4)
packages/conform-react/future/types.ts (1)
646-676: LGTM!Also applies to: 713-778
packages/conform-react/tests/types.test-d.ts (1)
171-288: LGTM!packages/conform-react/tests/useControl.browser.test.tsx (1)
1125-1136: LGTM!.changeset/fair-intents-infer.md (1)
1-5: LGTM!
cf1347a to
ac37616
Compare
Summary
Testing
tsc --project packages/conform-react/tsconfig.build.jsonvitest --config packages/conform-react/vitest.config.ts --project "conform-react (node)" --run tests/types.test-d.ts tests/intent.test.ts