docs: clarify pipe behavior for missing keys in optional schemas#1304
docs: clarify pipe behavior for missing keys in optional schemas#1304fabian-hiller merged 7 commits intoopen-circle:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR clarifies the behavior of pipe execution when using optional schemas (optional and exactOptional) with missing object keys. The documentation explains that pipe actions only execute when default values are provided or keys are present.
- Added comprehensive documentation about pipe execution behavior for missing keys
- Enhanced API documentation for
optionalandexactOptionalschemas with pipe examples - Improved clarity around when transformations and other pipe actions execute
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| website/src/routes/guides/(schemas)/optionals/index.mdx | Added new section explaining pipe execution behavior with and without default values |
| website/src/routes/api/(schemas)/optional/index.mdx | Added important note and examples showing pipe behavior for missing keys |
| website/src/routes/api/(schemas)/exactOptional/index.mdx | Added similar important note and pipe examples for exact optional schemas |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Add important notes about pipe execution for missing keys - Explain that default values are required for pipe execution - Improve clarity across optional, exactOptional, and guides - Enhance understanding of transformation behavior
|
I haven't forgotten you and will probably merge this PR in 1 to 2 weeks. I am currently focused on Formisch a few other things. |
|
It is still on my list. Sorry for the delay. |
|
@fabian-hiller is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughDocumentation was updated across multiple MDX pages (optional, exactOptional, nullish and their async variants) to clarify object-schema behavior: when an object key is missing, the entry’s 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
website/src/routes/guides/(schemas)/optionals/index.mdx (1)
130-130: Make the final sentence unambiguous.Line 130’s “This behavior” is vague after two opposite cases. Consider explicitly saying “With
default_provided…” to avoid ambiguity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/routes/guides/`(schemas)/optionals/index.mdx at line 130, The closing sentence is ambiguous; change it to explicitly state the condition by referring to the default_ parameter (e.g., start the sentence "With `default_` provided, ...") so readers know which case you're describing; update the sentence that currently begins "This behavior ensures..." to begin with "With `default_` provided, this behavior ensures..." (or equivalent) so the meaning is unambiguous and clearly tied to the `default_` behavior described earlier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/routes/guides/`(schemas)/optionals/index.mdx:
- Around line 94-99: The paragraph claiming "the pipe still runs" needs to be
scoped to each wrapper: update the copy to state that for present null or
undefined values the behavior differs by wrapper—`optional` and `nullish`
perform an early-return and therefore skip the inner `pipe` when the value is
`undefined` (and `null` for `nullish`), whereas `exactOptional` delegates
directly to its wrapped schema so the wrapped `pipe` is executed for present
`undefined` values; keep the note that missing keys are ignored when no
`default_` is provided. Reference `optional`, `exactOptional`, `nullish`,
`pipe`, and `default_` in the revised text.
---
Nitpick comments:
In `@website/src/routes/guides/`(schemas)/optionals/index.mdx:
- Line 130: The closing sentence is ambiguous; change it to explicitly state the
condition by referring to the default_ parameter (e.g., start the sentence "With
`default_` provided, ...") so readers know which case you're describing; update
the sentence that currently begins "This behavior ensures..." to begin with
"With `default_` provided, this behavior ensures..." (or equivalent) so the
meaning is unambiguous and clearly tied to the `default_` behavior described
earlier.
🪄 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: CHILL
Plan: Pro
Run ID: 9afbc980-22a9-496a-9313-8d528b0c85de
📒 Files selected for processing (7)
website/src/routes/api/(async)/exactOptionalAsync/index.mdxwebsite/src/routes/api/(async)/nullishAsync/index.mdxwebsite/src/routes/api/(async)/optionalAsync/index.mdxwebsite/src/routes/api/(schemas)/exactOptional/index.mdxwebsite/src/routes/api/(schemas)/nullish/index.mdxwebsite/src/routes/api/(schemas)/optional/index.mdxwebsite/src/routes/guides/(schemas)/optionals/index.mdx
✅ Files skipped from review due to trivial changes (6)
- website/src/routes/api/(schemas)/optional/index.mdx
- website/src/routes/api/(async)/exactOptionalAsync/index.mdx
- website/src/routes/api/(schemas)/exactOptional/index.mdx
- website/src/routes/api/(async)/optionalAsync/index.mdx
- website/src/routes/api/(async)/nullishAsync/index.mdx
- website/src/routes/api/(schemas)/nullish/index.mdx
| When an object entry uses <Link href="/api/optional/">`optional`</Link>, <Link href="/api/exactOptional/">`exactOptional`</Link>, or <Link href="/api/nullish/">`nullish`</Link> inside a <Link href="/api/pipe/">`pipe`</Link>, it is important to understand when the pipe executes. | ||
|
|
||
| ### Without default values | ||
|
|
||
| If no `default_` value is provided, missing object keys are completely ignored and their pipes will **not** be executed. This only applies to missing keys. If the key is present with `null` or `undefined`, the pipe still runs. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification of exactOptional runtime semantics and docs alignment.
set -euo pipefail
# 1) Confirm exactOptional run path
rg -n -C3 "exactOptional|~run|return this\\.wrapped\\['~run'\\]" library/src/schemas/exactOptional
# 2) Compare optional/nullish early-return behavior
rg -n -C4 "If value is|return dataset as SuccessDataset" library/src/schemas/optional library/src/schemas/nullish
# 3) Locate existing docs/examples discussing exactOptional + pipe + undefined
rg -n -C3 "exactOptional|pipe|default_|undefined" website/src/routes/api website/src/routes/guidesRepository: open-circle/valibot
Length of output: 50379
Scope the "pipe still runs" claim per wrapper type.
Line 98 currently presents behavior as universal for optional, exactOptional, and nullish, but they have different runtime semantics for present undefined values. optional and nullish skip the pipe via early-return, while exactOptional delegates to the wrapped schema without early-return logic.
Suggested revision
-If no `default_` value is provided, missing object keys are completely ignored and their pipes will **not** be executed. This only applies to missing keys. If the key is present with `null` or `undefined`, the pipe still runs.
+If no `default_` value is provided, missing object keys are ignored and their pipes will **not** be executed.
+For present keys, behavior depends on the wrapper:
+- `optional` / `nullish`: early-returns on `undefined`/`null` (pipe skipped).
+- `exactOptional`: no early-return; present `undefined` is validated by the wrapped schema.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| When an object entry uses <Link href="/api/optional/">`optional`</Link>, <Link href="/api/exactOptional/">`exactOptional`</Link>, or <Link href="/api/nullish/">`nullish`</Link> inside a <Link href="/api/pipe/">`pipe`</Link>, it is important to understand when the pipe executes. | |
| ### Without default values | |
| If no `default_` value is provided, missing object keys are completely ignored and their pipes will **not** be executed. This only applies to missing keys. If the key is present with `null` or `undefined`, the pipe still runs. | |
| When an object entry uses <Link href="/api/optional/">`optional`</Link>, <Link href="/api/exactOptional/">`exactOptional`</Link>, or <Link href="/api/nullish/">`nullish`</Link> inside a <Link href="/api/pipe/">`pipe`</Link>, it is important to understand when the pipe executes. | |
| ### Without default values | |
| If no `default_` value is provided, missing object keys are ignored and their pipes will **not** be executed. | |
| For present keys, behavior depends on the wrapper: | |
| - `optional` / `nullish`: early-returns on `undefined`/`null` (pipe skipped). | |
| - `exactOptional`: no early-return; present `undefined` is validated by the wrapped schema. |
🧰 Tools
🪛 LanguageTool
[style] ~98-~98: Consider using a different adverb to strengthen your wording.
Context: ...ue is provided, missing object keys are completely ignored and their pipes will not be...
(COMPLETELY_ENTIRELY)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/routes/guides/`(schemas)/optionals/index.mdx around lines 94 -
99, The paragraph claiming "the pipe still runs" needs to be scoped to each
wrapper: update the copy to state that for present null or undefined values the
behavior differs by wrapper—`optional` and `nullish` perform an early-return and
therefore skip the inner `pipe` when the value is `undefined` (and `null` for
`nullish`), whereas `exactOptional` delegates directly to its wrapped schema so
the wrapped `pipe` is executed for present `undefined` values; keep the note
that missing keys are ignored when no `default_` is provided. Reference
`optional`, `exactOptional`, `nullish`, `pipe`, and `default_` in the revised
text.
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
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="website/src/routes/guides/(schemas)/optionals/index.mdx">
<violation number="1" location="website/src/routes/guides/(schemas)/optionals/index.mdx:98">
P2: The note "If the key is present with `null` or `undefined`, the pipe still runs" is an oversimplification that is inaccurate for the schema types discussed. `optional(v.string())` rejects `null`, and `exactOptional(v.string())` rejects `undefined` as a value. Consider qualifying which values each schema type accepts, e.g.:
> This only applies to missing keys. If the key is present, the pipe runs as long as the value passes the inner schema validation.</violation>
</file>
<file name="website/src/routes/api/(schemas)/nullish/index.mdx">
<violation number="1" location="website/src/routes/api/(schemas)/nullish/index.mdx:94">
P2: The output type comment likely includes `| null` incorrectly. The `v.transform` always returns a `string` (via `(input ?? 'hello').toUpperCase()`), so `null` is eliminated from the pipe's output type. Compare with the analogous example on the `optional` page, which correctly shows `{ isActive?: boolean | undefined }` without `| null`. This should be `{ value?: string | undefined }`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Uh oh!
There was an error while loading. Please reload this page.