perf(types): use key remapping in MarkOptional by fabian-hiller · Pull Request #1675 · open-circle/valibot · GitHub
Skip to content

perf(types): use key remapping in MarkOptional - #1675

Open
fabian-hiller wants to merge 2 commits into
perf/types-variant-option-constraintfrom
perf/types-mark-optional-key-remapping
Open

fabian-hiller wants to merge 2 commits into
perf/types-variant-option-constraintfrom
perf/types-mark-optional-key-remapping

Conversation

@fabian-hiller

Copy link
Copy Markdown
Member

Part of #1663 (finding 10). Stacked on #1670. On main alone, this change breaks the variant tests for optional discriminators. The current variant constraint relies on how Omit treats the known keys of index-signature types (it makes them optional), and #1670 replaces that constraint.

This changes how types are displayed, so it needs a decision.

Change

MarkOptional now uses two key-remapped mapped types (the same approach as Zod v4's $InferObjectOutput):

export type MarkOptional<TObject, TKeys extends keyof TObject> = {
  [TKey in keyof TObject as TKey extends TKeys ? never : TKey]: TObject[TKey];
} & {
  [TKey in keyof TObject as TKey extends TKeys ? TKey : never]?: TObject[TKey];
};

It used to be { [TKey in keyof TObject]?: unknown } & Omit<TObject, TKeys> & Partial<Pick<TObject, TKeys>>, and Omit in turn builds Pick + Exclude.

Measurements

TypeScript 6.0.3, schema-benchmarks probe, compared with #1670 (schema / infer):

Case #1670 This PR
Product benchmark from #1663 7,889 / 645 7,437 / 434
Object with optional entries 9,130 / 601 8,840 / 468
Mixed schema 12,504 / 3,441 11,449 / 3,516
partial/pick/omit/required 24,387 / 404 19,296 / 243

Behavior change

Assignability is unchanged: all runtime and type tests pass (toEqualTypeOf ignores key order). The displayed key order changes: required keys are listed before optional ones. For example, { id: number; email?: string; name: string } is now shown as { id: number; name: string; email?: string }. The current code comment says the { [TKey in keyof TObject]?: unknown } member exists to preserve key order, so this is a deliberate trade-off. An order-preserving variant, which keeps that member and only swaps Omit/Pick for key remapping, gave mixed results in my measurements.

🤖 Generated with Claude Code

fabian-hiller and others added 2 commits September 26, 2026 20:12
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 201a5103-a149-4e1b-a44e-5ae9d824ad21

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/valibot@1675

commit: 035dc64

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="library/src/types/utils.ts">

<violation number="1" location="library/src/types/utils.ts:64">
P3: This reimplementation intentionally changes the displayed key order of `Prettify`-wrapped output types (required keys now precede optional ones, e.g. `{ id: number; name: string; email?: string }`), but neither the code nor the CHANGELOG entry records the trade-off, and the comment explaining key-order preservation was deleted. Because `toEqualTypeOf` ignores key order, the test suite cannot catch this user-visible change to hover previews and docs. Add a comment above `MarkOptional` documenting the required-before-optional ordering, and if the order change is accepted, reflect it in the CHANGELOG entry.</violation>

<violation number="2" location="library/src/types/utils.ts:66">
P2: `MarkOptional` drops explicitly declared keys hidden by an index signature. Remap the optional half over `TKeys` directly so `MarkOptional<{ [key: string]: unknown; type: 'foo' }, 'type'>` still enforces `type?: 'foo'`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

export type MarkOptional<TObject, TKeys extends keyof TObject> = {
[TKey in keyof TObject as TKey extends TKeys ? never : TKey]: TObject[TKey];
} & {
[TKey in keyof TObject as TKey extends TKeys ? TKey : never]?: TObject[TKey];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: MarkOptional drops explicitly declared keys hidden by an index signature. Remap the optional half over TKeys directly so MarkOptional<{ [key: string]: unknown; type: 'foo' }, 'type'> still enforces type?: 'foo'.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At library/src/types/utils.ts, line 66:

<comment>`MarkOptional` drops explicitly declared keys hidden by an index signature. Remap the optional half over `TKeys` directly so `MarkOptional<{ [key: string]: unknown; type: 'foo' }, 'type'>` still enforces `type?: 'foo'`.</comment>

<file context>
@@ -60,10 +60,11 @@ export type Prettify<TObject> = { [TKey in keyof TObject]: TObject[TKey] } & {};
+export type MarkOptional<TObject, TKeys extends keyof TObject> = {
+  [TKey in keyof TObject as TKey extends TKeys ? never : TKey]: TObject[TKey];
+} & {
+  [TKey in keyof TObject as TKey extends TKeys ? TKey : never]?: TObject[TKey];
+};
 
</file context>
Suggested change
[TKey in keyof TObject as TKey extends TKeys ? TKey : never]?: TObject[TKey];
[TKey in TKeys]?: TObject[TKey];

{ [TKey in keyof TObject]?: unknown } & Omit<TObject, TKeys> &
Partial<Pick<TObject, TKeys>>;
export type MarkOptional<TObject, TKeys extends keyof TObject> = {
[TKey in keyof TObject as TKey extends TKeys ? never : TKey]: TObject[TKey];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: This reimplementation intentionally changes the displayed key order of Prettify-wrapped output types (required keys now precede optional ones, e.g. { id: number; name: string; email?: string }), but neither the code nor the CHANGELOG entry records the trade-off, and the comment explaining key-order preservation was deleted. Because toEqualTypeOf ignores key order, the test suite cannot catch this user-visible change to hover previews and docs. Add a comment above MarkOptional documenting the required-before-optional ordering, and if the order change is accepted, reflect it in the CHANGELOG entry.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At library/src/types/utils.ts, line 64:

<comment>This reimplementation intentionally changes the displayed key order of `Prettify`-wrapped output types (required keys now precede optional ones, e.g. `{ id: number; name: string; email?: string }`), but neither the code nor the CHANGELOG entry records the trade-off, and the comment explaining key-order preservation was deleted. Because `toEqualTypeOf` ignores key order, the test suite cannot catch this user-visible change to hover previews and docs. Add a comment above `MarkOptional` documenting the required-before-optional ordering, and if the order change is accepted, reflect it in the CHANGELOG entry.</comment>

<file context>
@@ -60,10 +60,11 @@ export type Prettify<TObject> = { [TKey in keyof TObject]: TObject[TKey] } & {};
-  { [TKey in keyof TObject]?: unknown } & Omit<TObject, TKeys> &
-    Partial<Pick<TObject, TKeys>>;
+export type MarkOptional<TObject, TKeys extends keyof TObject> = {
+  [TKey in keyof TObject as TKey extends TKeys ? never : TKey]: TObject[TKey];
+} & {
+  [TKey in keyof TObject as TKey extends TKeys ? TKey : never]?: TObject[TKey];
</file context>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="library/CHANGELOG.md">

<violation number="1" location="library/CHANGELOG.md:7">
P3: This PR changes how inferred object types are displayed for every schema with optional entries: required keys are now shown before optional keys (e.g. `{ id: number; name: string; email?: string }` vs the original declaration order), because `Prettify` resolves the two key-remapped members in intersection order and the old `{ [TKey in keyof TObject]?: unknown }` order-preserving member is gone. Since the old comment documented that member as existing precisely to preserve key order, record this user-visible display change in the CHANGELOG entry (or keep the order-preserving member) so the decision called for in the PR is explicit.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread library/CHANGELOG.md

## vX.X.X (Month DD, YYYY)

- Change `MarkOptional` type to use key remapping instead of `Omit`, `Pick` and `Partial` to improve TypeScript performance (pull request #1675)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: This PR changes how inferred object types are displayed for every schema with optional entries: required keys are now shown before optional keys (e.g. { id: number; name: string; email?: string } vs the original declaration order), because Prettify resolves the two key-remapped members in intersection order and the old { [TKey in keyof TObject]?: unknown } order-preserving member is gone. Since the old comment documented that member as existing precisely to preserve key order, record this user-visible display change in the CHANGELOG entry (or keep the order-preserving member) so the decision called for in the PR is explicit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At library/CHANGELOG.md, line 7:

<comment>This PR changes how inferred object types are displayed for every schema with optional entries: required keys are now shown before optional keys (e.g. `{ id: number; name: string; email?: string }` vs the original declaration order), because `Prettify` resolves the two key-remapped members in intersection order and the old `{ [TKey in keyof TObject]?: unknown }` order-preserving member is gone. Since the old comment documented that member as existing precisely to preserve key order, record this user-visible display change in the CHANGELOG entry (or keep the order-preserving member) so the decision called for in the PR is explicit.</comment>

<file context>
@@ -4,6 +4,7 @@ All notable changes to the library will be documented in this file.
 
 ## vX.X.X (Month DD, YYYY)
 
+- Change `MarkOptional` type to use key remapping instead of `Omit`, `Pick` and `Partial` to improve TypeScript performance (pull request #1675)
 - Change internal `VariantOption` and `VariantOptionAsync` types of `variant` schema to use a structural object option type to improve TypeScript performance (pull request #1670)
 - Change `isbn`, `creditCard` and `imei` actions to check digits by character code instead of splitting and parsing the input to improve performance (pull request #1634)
</file context>
Suggested change
- Change `MarkOptional` type to use key remapping instead of `Omit`, `Pick` and `Partial` to improve TypeScript performance (pull request #1675)
- Change `MarkOptional` type to use key remapping instead of `Omit`, `Pick` and `Partial` to improve TypeScript performance. This also changes the order of keys in inferred types - required keys are now shown before optional keys (pull request #1675)

@yslpn

yslpn commented Sep 29, 2026

Copy link
Copy Markdown
Member

This branch has not been deployed

No deployments
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