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(string): Create .datetime() #2087

Merged
merged 5 commits into from
Feb 28, 2024
Merged

feat(string): Create .datetime() #2087

merged 5 commits into from
Feb 28, 2024

Conversation

0livare
Copy link
Contributor

@0livare 0livare commented Aug 16, 2023

yup.date() always attempts to parse input into a Date object. Sometimes that is undesirable and you just want the string to be left alone. Currently yup does not have a solution for that use case, but that's where .datetime() comes in!

With an API inspired by zod.datetime, datetime also provides greater control over the exact required format of the ISO string than date does.

@0livare
Copy link
Contributor Author

0livare commented Sep 27, 2023

@jquense is this something you would consider?


Unlike `.date()`, `datetime` will not convert the string to a `Date` object. `datetime` also provides greater customization over the required format of the datetime string than `date` does.

`options.allowOffset`: Allow a time zone offset. False requires UTC 'Z' timezone. _(default: false)_
Copy link
Owner

Choose a reason for hiding this comment

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

why default to false for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking was that most of the time when you store an iso date in a DB (which in my mind will be the most common use case for this function) you would want them to be consistently formatted in UTC time for easy comparison.

src/string.ts Outdated
/** Allow a time zone offset. False requires UTC 'Z' timezone. (default: false) */
allowOffset?: boolean | null;
/** Require a certain sub-second precision on the date. (default: null -- any or no sub-second precision) */
precision?: number | null;
Copy link
Owner

Choose a reason for hiding this comment

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

i'd have both of these options not be nullable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@jquense
Copy link
Owner

jquense commented Sep 29, 2023

yes open to this 👍

@0livare
Copy link
Contributor Author

0livare commented Dec 7, 2023

@jquense Just a heads up that I believe I have addressed both of your comments!

@jquense
Copy link
Owner

jquense commented Feb 1, 2024

@0livare sorry this is taking so long but can you fix the build?

This will allow us to reuse the date struct for .datetime()
undefined is now the default value for precision, which I think makes
sense because it indicates that the precision is "not defined", and
therefore can have any or zero digits of precision.
@0livare
Copy link
Contributor Author

0livare commented Feb 14, 2024

@jquense build is fixed! (no changes, I just rebased onto the latest master)

@jquense jquense merged commit 2a9e060 into jquense:master Feb 28, 2024
1 check passed
@jquense
Copy link
Owner

jquense commented Feb 28, 2024

thanks!

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