Skip to content
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

feat(actions): timestamps with optional "T" #1064

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

emilesabatier
Copy link

@emilesabatier emilesabatier commented Feb 24, 2025

Hi,

According to ISO 8601 (cf. https://en.wikipedia.org/wiki/ISO_8601#Times), the "T" character may be optional in unambiguous contexts.

ISO 8601-1:2019 allows the T to be omitted in the extended format, as in "13:47:30", but only allows the T to be omitted in the basic format when there is no risk of confusion with date expressions.

I encountered the issue using pipe validation on data from postgresql + drizzle, where the iso timestamp string is returned without the "T" letter.

Maybe a discussion is needed on the "unambiguous" part ?

Copy link

vercel bot commented Feb 24, 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 Feb 24, 2025 3:05pm

@fabian-hiller
Copy link
Owner

Thank you for creating this PR. Can you share the schema that led to this issue?

@fabian-hiller fabian-hiller self-assigned this Feb 26, 2025
@fabian-hiller fabian-hiller added the question Further information is requested label Feb 26, 2025
@emilesabatier
Copy link
Author

emilesabatier commented Feb 26, 2025

Sure, here is the code sample

import { timestamp } from "drizzle-orm/pg-core"
import * as v from "valibot"

// Drizzle column definition, returns "2025-02-26 12:42:03+00" string from database
const timestampColumn = timestamp("created_on", { mode: "string", withTimezone: true, precision: 0 })

// Valibot schema for that column
const timestampSchema = v.pipe(
    v.string(),
    v.isoTimestamp()
)

I have seen a related issue in drizzle repo : drizzle-team/drizzle-orm#4150

@fabian-hiller
Copy link
Owner

I reached out to the Drizzle team. I will get back to you in the next few days.

@fabian-hiller
Copy link
Owner

I have received an answer. This seems to be a database related issue. Drizzle may be planning to address this issue in the long term. What database are you using? I will have to do some research and then decide if we want to change our current regex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants