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

toJSON throw exception instead of returns null #632

Closed
guillaumebriday opened this issue Jul 15, 2019 · 4 comments
Closed

toJSON throw exception instead of returns null #632

guillaumebriday opened this issue Jul 15, 2019 · 4 comments

Comments

@guillaumebriday
Copy link
Contributor

Describe the bug
There is a different behaviour between moment and dayjs when date is invalid and toJSON is called.

dayjs('wtf').toJSON()
> Uncaught RangeError: Invalid time value
moment('wtf').toJSON()
> null

Expected behavior
It's annoying when you when to call an api with a nullable field, for example :

this.$store.dispatch('tasks/addTask', {
    title: this.form.title,
    due_at: dayjs(this.form.due_at).second(0)
})

This code does not work if this.form.due_at is "" or null

But this one works :

this.$store.dispatch('tasks/addTask', {
    title: this.form.title,
    due_at: moment(this.form.due_at).second(0)
})

Thank you in advance

Information

  • Day.js Version 1.8.15
  • OS: macOS 10.14.5
  • Browser chrome 75 and firefox 68
  • Time zone: Paris
@ghost
Copy link

ghost commented Jul 16, 2019

https://runkit.com/embed/tay2fpsxv5bz

moment('').second(0) moment(null).second(0) also return Invalid Date

@g1eny0ung
Copy link
Contributor

g1eny0ung commented Jul 16, 2019

@xxyuk I think guillaumebriday means that the toJSON() method would stop the program.

I view the moment.js' source code. It will check the date is valid then choose to call toISOString() or return null.

function toJSON () {
  // new Date(NaN).toJSON() === null
  return this.isValid() ? this.toISOString() : null;
}

Should we also check its validity first by calling isValid()?

@guillaumebriday
Copy link
Contributor Author

yep exactly @g1eny0ung !

I ended doing this in my own application :

{ due_at: dayjs(this.form.due_at).isValid() ? dayjs(this.form.due_at).second(0) : null }

I will open a PR

@g1eny0ung
Copy link
Contributor

@guillaumebriday 👍

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

No branches or pull requests

2 participants