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

fix: string date format validation #484

Merged
merged 2 commits into from
Jul 8, 2022

Conversation

ivan-tymoshenko
Copy link
Member

@ivan-tymoshenko ivan-tymoshenko commented Jul 2, 2022

Checklist

@ivan-tymoshenko
Copy link
Member Author

Please make sure that this test works as expected.

test('Validate Date object as string type', (t) => {

}
return this.asString(date)
throw new Error(`The value "${date}" cannot be converted to a date-time.`)
Copy link
Member

Choose a reason for hiding this comment

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

What if I passed a string that fulfill date-time, date, time?
It seems like it only accept Date object after this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you pass a string it will be stringified in the asString method.

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

LGTM.

@climba03003 climba03003 requested a review from Eomm July 5, 2022 11:42
return validateTimeFormat(date)
}
return false
keyword: 'fjs_type',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
keyword: 'fjs_type',
keyword: 'fjs_date_type',

I think we should try not to change this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mcollina I agree, but it is a completely different meaning of this property now. It's not a date format anymore. It describes how ajv should validate custom type at all. For example, for string type, there would be not only the Date object validation but also Regexp objects and some other exceptions.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Eomm Eomm merged commit 967bacf into master Jul 8, 2022
@Fdawgs Fdawgs deleted the date-format-with-string-optional-type branch July 31, 2022 06:46
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.

nullable doesn't work in schemas using format and anyOf
4 participants