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

Replace momentjs by date-fns #37

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

Conversation

Izabrr
Copy link

@Izabrr Izabrr commented Mar 10, 2021

No description provided.

Copy link
Contributor

@kanongil kanongil left a 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).
Copy link
Contributor

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());
Copy link
Contributor

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');
Copy link
Contributor

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');
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad example

@kanongil
Copy link
Contributor

I would probably have a look at Luxon, which is a sort of successor to moment:
https://moment.github.io/luxon/docs/manual/parsing.html#table-of-tokens

@hueniverse
Copy link
Contributor

@Izabrr planning to address @kanongil comments?

@smujaddid
Copy link

Or use dayjs instead, the API is much closer I think

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.

None yet

4 participants