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

An unexpected type returned from dayjs.minMax when called with empty array #1061

Closed
dearlordylord opened this issue Sep 20, 2020 · 7 comments · Fixed by #1062
Closed

An unexpected type returned from dayjs.minMax when called with empty array #1061

dearlordylord opened this issue Sep 20, 2020 · 7 comments · Fixed by #1062
Labels

Comments

@dearlordylord
Copy link

Describe the bug

An empty array is very unexpected here:

typeof dayjs.max([]) === 'array'; // true

there's an example:

https://jsfiddle.net/7y5f6r3g/1/

Expected behavior

I expect to see either falsy value or a thrown exception when dayjs.max is called with an empty array.

Information

  • Day.js Version 1.8.36
  • OS: OSX
  • Browser Chrome
@iamkun
Copy link
Owner

iamkun commented Sep 21, 2020

Thanks, we will fix it to 'Return current time if passing empty array' the same as 'Return current time if no argument'

@dearlordylord
Copy link
Author

@iamkun thank you for checking it so quickly! Appreciated.

However, I humbly ask you to reconsider returning the current date. Had the library returned the current date I wouldn't have never caught this bug, leaving it to users instead to be puzzled by weird component behaviour. With an array, I at least had a chance to get an exception during development.

In that sense, there's no difference between returning the current date, 1 January 1970 or Donald Trump's birthday - all lead to hard to catch bug, with no help from static typing (if ts is used) or from dynamic type system (i.e. this array I've encountered giving some 'is not a function' error).

@iamkun
Copy link
Owner

iamkun commented Sep 26, 2020

Thanks, that makes sense.

Maybe Return null will be better to "passing empty array" and "passing no argument"?

@dearlordylord
Copy link
Author

Whatever fits better the library general error handling approach. Assuming your API is similar to moment.js, they return null for this case. Also, a lot of other JS libraries would do so in similar cases. I think returning null is a safe bet.

@iamkun
Copy link
Owner

iamkun commented Sep 27, 2020

https://runkit.com/embed/c65l73q16y8p

image

An interesting result from moment.js

@iamkun
Copy link
Owner

iamkun commented Oct 23, 2020

Finally, decide to return null while "passing empty array" and "passing no argument"?

@iamkun iamkun closed this as completed Oct 23, 2020
@iamkun
Copy link
Owner

iamkun commented Oct 23, 2020

🎉 This issue has been resolved in version 1.9.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants