-
Notifications
You must be signed in to change notification settings - Fork 25
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
Replace momentjs by date-fns #37
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some quick comments.
The feature does seem thought through. date-fns
allows a lot more formatting options, and a lot of it is locale dependent, which is impossible to configure. Additionally, it provides no time zone support.
``` | ||
|
||
## Rules | ||
|
||
### `date.format(format)` | ||
|
||
Specifies the allowed date format: | ||
- `format` - string or array of strings that follow the `moment.js` [format](http://momentjs.com/docs/#/parsing/string-format/). | ||
- `format` - string that follows the `date-fns` [format](https://date-fns.org/v2.19.0/docs/parse). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably link to: https://date-fns.org/docs/parse
method: function (enabled = true) { | ||
|
||
return this.$_setFlag('utc', enabled); | ||
const date = Parse(value, format, new Date()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling different results depending in the time of invocation does not seem like a good idea.
@@ -8,27 +8,16 @@ This version requires **joi** v17 or newer. | |||
const Joi = require('joi') | |||
.extend(require('@joi/date')); | |||
|
|||
const schema = Joi.date().format('YYYY-MM-DD').utc(); | |||
const schema = Joi.date().format('YYYY-MM-DD'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad example, you are parsing 'local week-numbering year' and 'day of a year', which doesn't make sense.
|
||
```js | ||
const schema = Joi.date().format(['YYYY/MM/DD', 'DD-MM-YYYY']); | ||
``` | ||
```js | ||
const schema = Joi.date().format('YYYY-MM-DD HH:mm'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad example
I would probably have a look at Luxon, which is a sort of successor to moment: |
Or use dayjs instead, the API is much closer I think |
No description provided.