Skip to content

feat: create coercion actions for primitives (and date) #1212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

EskiMojo14
Copy link
Contributor

Decisions left:

  • should we try/catch around conversions? it's rare but some can throw, like Number(Symbol())
  • v.toNumber - should we check for NaN after converting, or recommend piping into v.number?
  • v.toDate - similarly, should we check the date is valid, or recommend piping into v.date?

Copy link

vercel bot commented May 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
valibot ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 6:43pm

@fabian-hiller
Copy link
Owner

Thank you so much!

should we try/catch around conversions? it's rare but some can throw, like Number(Symbol())

Yes, we should do that whenever it is necessary to cover all edge cases

v.toNumber - should we check for NaN after converting, or recommend piping into v.number?
v.toDate - similarly, should we check the date is valid, or recommend piping into v.date?

I would only convert it. Do you know what Zod is doing here?

@EskiMojo14
Copy link
Contributor Author

with Zod the coercion isn't separate from the schema (it just happens as the first step of the schema if configured), so z.coerce.number() would be the same as v.pipe(v.unknown(), v.toNumber(), v.number())

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a set of conversion actions for primitives—including string, number, date, boolean, and bigint—along with comprehensive tests and type validations.

  • Introduces conversion functions for each type with try-catch logic for error handling
  • Adds tests to validate both successful coercions and proper issue reporting on invalid inputs
  • Updates re-export files to include the newly added actions

Reviewed Changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
library/src/actions/toString/* Adds toString conversion action and tests
library/src/actions/toNumber/* Introduces toNumber conversion with error handling
library/src/actions/toDate/* Implements toDate conversion with try-catch (see note)
library/src/actions/toBoolean/* Adds toBoolean conversion action and tests
library/src/actions/toBigint/* Introduces toBigint conversion with error handling
library/src/actions/index.ts Updates exports to include the new conversion actions

Comment on lines +56 to +60
dataset.value = new Date(dataset.value);
} catch {
_addIssue(this, 'date', dataset, config);
// @ts-expect-error
dataset.typed = false;
Copy link
Preview

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

new Date() does not throw an exception for invalid date strings; consider validating the resulting Date (e.g., checking if isNaN(dataset.value.getTime())) and registering an issue if it's invalid.

Suggested change
dataset.value = new Date(dataset.value);
} catch {
_addIssue(this, 'date', dataset, config);
// @ts-expect-error
dataset.typed = false;
const parsedDate = new Date(dataset.value);
if (isNaN(parsedDate.getTime())) {
_addIssue(this, 'date', dataset, config);
// @ts-expect-error
dataset.typed = false;
} else {
dataset.value = parsedDate;
}

Copilot uses AI. Check for mistakes.

Copy link
Owner

Choose a reason for hiding this comment

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

I think right now we want devs to explicitly handle this case with v.pipe(v.unknown(), v.toDate(), v.date()).

@fabian-hiller fabian-hiller added this to the v1.2 milestone Jun 11, 2025
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants