fix(conform-react): preserve string literal types in DefaultValue - #1281
fix(conform-react): preserve string literal types in DefaultValue#1281transparent-citizen wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 822dbdf 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesDefaultValue literal preservation
Estimated code review effort: 2 (Simple) | ~10 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: |
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 198-200: Update the conditional type branch in DefaultValue<Shape>
so nullable or undefined constituents are handled separately while the
non-nullish string subset remains literal-preserving. Ensure a Shape such as {
intent?: 'save' | 'publish' } does not widen to string and therefore rejects
unsupported values like 'delete'.
🪄 Autofix (Beta)
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: f601dce6-da2a-4d56-a3f5-aa6bf067d5ad
📒 Files selected for processing (3)
.changeset/olive-trees-narrow.mdpackages/conform-react/future/types.tspackages/conform-react/tests/types.test-d.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: Release
- GitHub Check: E2E Tests (macos-latest, chromium, 22)
- GitHub Check: E2E Tests (ubuntu-latest, chromium, 22)
- GitHub Check: Future API Tests (ubuntu-latest, webkit, 22)
- GitHub Check: Future API Tests (ubuntu-latest, chromium, 22)
- GitHub Check: API Tests (v1)
- GitHub Check: E2E Tests (ubuntu-latest, firefox, 22)
- GitHub Check: Future API Tests (windows-latest, chromium, 22)
- GitHub Check: Future API Tests (ubuntu-latest, firefox, 22)
- GitHub Check: Future API Tests (macos-latest, chromium, 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: Future API Tests (macos-latest, webkit, 22)
🧰 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/olive-trees-narrow.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (2)
packages/conform-react/tests/types.test-d.ts (1)
123-157: LGTM!.changeset/olive-trees-narrow.md (1)
1-5: LGTM!
0c305ec to
4adec14
Compare
|
The examples are making use of unvalidated search params in In the real world, we would typically validate all search params before use to prevent unsound application state. In the examples, I think that a type assertion is good enough to get the point across. By doing that, we're making the promise to TypeScript that these search params will have the correct type, even though TypeScript can show that there are ways for the type to be wider at runtime. |
083b78e to
251d9e6
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 `@examples/radix-ui/src/App.tsx`:
- Around line 37-45: Normalize query-string defaults before constructing
defaultValue: in examples/radix-ui/src/App.tsx lines 37-45, validate carType,
userCountry, and desiredContractType against their schema literals and reject
unsupported values; in examples/headless-ui/src/App.tsx lines 61-62, validate
priority; in examples/react-aria/src/App.tsx lines 48-49, validate language and
every colors element; and in examples/shadcn-ui/src/App.tsx lines 76-86,
validate gender, job, accountType, categories, and nested object members.
Replace unsafe casts with the existing schema or equivalent type guards so
invalid URL values become the appropriate absent/default state.
🪄 Autofix (Beta)
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: f0318bef-dc10-40fc-82c2-dc1f01722e65
📒 Files selected for processing (4)
examples/headless-ui/src/App.tsxexamples/radix-ui/src/App.tsxexamples/react-aria/src/App.tsxexamples/shadcn-ui/src/App.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (11)
- GitHub Check: Release
- GitHub Check: Future API Tests (ubuntu-latest, firefox, 22)
- GitHub Check: E2E Tests (ubuntu-latest, firefox, 22)
- GitHub Check: Future API Tests (ubuntu-latest, webkit, 22)
- GitHub Check: Future API Tests (windows-latest, chromium, 22)
- GitHub Check: E2E Tests (ubuntu-latest, webkit, 22)
- GitHub Check: E2E Tests (ubuntu-latest, chromium, 22)
- GitHub Check: E2E Tests (windows-latest, chromium, 22)
- GitHub Check: E2E Tests (macos-latest, webkit, 22)
- GitHub Check: E2E Tests (macos-latest, chromium, 22)
- GitHub Check: API Tests (v1)
🔇 Additional comments (4)
examples/headless-ui/src/App.tsx (1)
36-37: LGTM!examples/radix-ui/src/App.tsx (1)
25-26: LGTM!examples/react-aria/src/App.tsx (1)
35-36: LGTM!examples/shadcn-ui/src/App.tsx (1)
45-46: LGTM!
|
Coderabbit got mad at me for proposing type assertions 😅 So I went and validated all search params. This should make the examples a little more streamlined, getting rid of the lengthy We now have type-safe default values in the examples, so working with string literals is a breeze. This also prevents invalid application state in the examples which was previously possible by constructing URLs that don't validate. The only downside is that this changes the param semantics in the examples from "use all, might lead to invalid application state" to "use valid only, otherwise start with empty form". I'm not a zod expert, but maybe |
|
The changeset was updated from |
I am not sure if that's something Conform can guarantee in the form levels. The default value could also be inferred from In fact, I have been meaning to make |
|
Hm, I see. The widening seems to have come up as a pain point in v1 before (#541), but I understand that #1284 allows simplifying the examples similarly, without overpromising type guarantees that it can't keep. But it also allows for unsound application state, since malformed URLs can transfer arbitrary data into the application. This is not a regression, it was possible before. Prior validation outside of I think #1284 is good to merge nonetheless. It feels like we're missing a validator-specific layer on top of |
|
Closing. This needs a validation layer somewhere. |
|
@transparent-citizen I think the narrowing you did is still useful. We can't narrow the output type (e.g. |
|
@edmundhung Just making sure I follow, is the suggestion to give the |
|
@transparent-citizen Changing the The case in #541 were an issue only because it impact how we infer |
This adds a case to
DefaultValueso that string literal types are preserved instead of being widened tostring, making fields like submit intents type-safe.The PR doesn't touch v1 in anticipation of its soon removal. If this is too soon, please don't hesitate to let me know, and I will retrofit it.
Generated Summary
DefaultValuenow preserves string literal types inconform-react/future, including optional, nullable, template-literal, and nested values. Other types retain existing behavior. Type tests cover these cases and reject incompatible literals. Examples cast URL-derived defaults to schema-specific types. A patch changeset was added.