Action isoTimestamp allows space in UTC offset#1195
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the ISO 8601 timestamp regex to accept a single space before the timezone offset for better compatibility with PostgreSQL’s timestamptz output.
- Allow an optional space before the UTC offset in
ISO_TIMESTAMP_REGEX. - Add tests covering timestamps with a space before the offset.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| library/src/regex.ts | Updated ISO_TIMESTAMP_REGEX to accept a leading space before +/- offset. |
| library/src/actions/isoTimestamp/isoTimestamp.test.ts | Added a test suite for timestamps with a space before the UTC offset. |
|
Thank you for creating this PR! Are you aware of any drawbacks? I'm a bit concerned because it doesn't seem to be ISO 8601 compliant. |
|
Thank you for asking! I agree this tweak isn’t strictly ISO 8601, but I don’t anticipate any serious drawbacks, mainly because it matches common real-world usage. For example, PostgreSQL’s Also, our current In short, this change improves compatibility with real data (avoiding false validation errors on perfectly valid SQL-standard timestamps) while staying aligned with the library’s goal of being practical and developer-friendly. |
1957b1d to
4abb005
Compare
|
Sorry for my late reply! I agree but I want to look at other schemas library like Zod first to see how they handle it. For now, I would plan to release this as a "fix" with Valibot v1.2. |
4abb005 to
c45337d
Compare
c45337d to
0e149f2
Compare
0e149f2 to
3fae6fa
Compare
|
@frenzzy is attempting to deploy a commit to the Valibot Team on Vercel. A member of the Team first needs to authorize it. |
|
Sorry for the delay! I will try to look into it when working on Valibot v1.3. |
|
@coderabbitai please review this PR |
🧩 Analysis chain🏁 Script executed: Length of output: 164 🌐 Web query: 💡 Result: PostgreSQL Zod: allow offsets (but still expects
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughISO timestamp validation was updated to accept an optional space before the timezone offset and to allow a space as the date/time separator; tests were added for space-separated UTC offsets and some test comments were realigned. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
library/src/regex.ts (1)
106-110: Consider updating JSDoc to document the PostgreSQL-style space allowance.The regex now accepts an optional space before the timezone offset (e.g.,
2023-07-11 17:26:27.243 +00:00), which deviates from strict ISO 8601. Since the library already takes a pragmatic approach (allowing space instead ofT), documenting this extension helps users understand the accepted formats.📝 Proposed JSDoc update
/** - * [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) timestamp regex. + * [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) timestamp regex. + * + * Hint: This regex allows a space instead of `T` as the date/time separator + * and an optional space before the timezone offset for PostgreSQL compatibility. */ export const ISO_TIMESTAMP_REGEX: RegExp =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library/src/regex.ts` around lines 106 - 110, Update the JSDoc for ISO_TIMESTAMP_REGEX to note the pragmatic extensions: mention that it accepts either 'T' or a space between date and time and explicitly allows an optional space before the timezone offset (PostgreSQL-style timestamps like "2023-07-11 17:26:27.243 +00:00"), clarifying that this deviates from strict ISO 8601; reference the exported constant ISO_TIMESTAMP_REGEX and keep the link to ISO 8601 for context.library/src/actions/isoTimestamp/isoTimestamp.test.ts (1)
130-139: Add test coverage forTseparator with space before offset.The new tests cover the PostgreSQL-style format (space as both date/time separator and before offset). However, the regex also accepts
Tas separator with a space before the offset (e.g.,2023-07-11T17:26:27.243 +00:00). Consider adding a test case to confirm this combination is intentionally allowed, or update the regex to disallow it if not intended.🧪 Proposed test addition
test('for space as separator in UTC offset', () => { expectNoActionIssue(action, [ '0000-01-01 00:00:00.000 +00:00', '0000-01-01 00:00:00.000 -00:00', '2023-07-11 17:26:27.243 +0000', '2023-07-11 17:26:27.243 -0000', '9999-12-31 23:59:59.999 +00', '9999-12-31 23:59:59.999 -00', + // T separator with space before offset + '2023-07-11T17:26:27.243 +12:34', + '2023-07-11T17:26:27.243 -12:34', ]); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library/src/actions/isoTimestamp/isoTimestamp.test.ts` around lines 130 - 139, Add an explicit test in isoTimestamp.test.ts to cover the combination of a literal "T" date/time separator with a space before the UTC offset (e.g. '2023-07-11T17:26:27.243 +00:00') so we verify whether that pattern is intentionally accepted; modify the existing test case using expectNoActionIssue(action, [...]) to include this string (or add a new test with the same helper) and if the behavior should be disallowed instead, update the ISO timestamp validation regex in the isoTimestamp validator to reject "T" followed by a space before the offset.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@library/src/actions/isoTimestamp/isoTimestamp.test.ts`:
- Around line 130-139: Add an explicit test in isoTimestamp.test.ts to cover the
combination of a literal "T" date/time separator with a space before the UTC
offset (e.g. '2023-07-11T17:26:27.243 +00:00') so we verify whether that pattern
is intentionally accepted; modify the existing test case using
expectNoActionIssue(action, [...]) to include this string (or add a new test
with the same helper) and if the behavior should be disallowed instead, update
the ISO timestamp validation regex in the isoTimestamp validator to reject "T"
followed by a space before the offset.
In `@library/src/regex.ts`:
- Around line 106-110: Update the JSDoc for ISO_TIMESTAMP_REGEX to note the
pragmatic extensions: mention that it accepts either 'T' or a space between date
and time and explicitly allows an optional space before the timezone offset
(PostgreSQL-style timestamps like "2023-07-11 17:26:27.243 +00:00"), clarifying
that this deviates from strict ISO 8601; reference the exported constant
ISO_TIMESTAMP_REGEX and keep the link to ISO 8601 for context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f2bfa07-4441-41a4-817b-80851f68d7f7
📒 Files selected for processing (2)
library/src/actions/isoTimestamp/isoTimestamp.test.tslibrary/src/regex.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
commit: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
Valibot v1.3 is available. Feel free to submit and expense of $15 for all your contributions for Valibot v1.3 via Open Collective. |
Fixes:
isoTimestamponly accepts UTC timestamps #495Changes:
+00:00).Reason:
timestamptzdefault output includes a space before the timezone offset:Summary by CodeRabbit
Bug Fixes
Tests
Documentation