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

format throws when used on "Invalid Date" with advancedformat + timezone + utc #1558

Open
SamuelPoulin opened this issue Jun 30, 2021 · 6 comments

Comments

@SamuelPoulin
Copy link

Describe the bug
When using the following format string on dayjs(null) the resulting date is "Invalid Date"

lll

When using the following string, dayjs throws

lll z

Thrown error:

Unhandled Runtime Error

RangeError: date value is not finite in DateTimeFormat.formatToParts()

dayjs(null).format('lll z')
           ^

Expected behavior
dayjs should display "Invalid Date" and not throw an error

Information

  • Day.js Version v1.10.5
  • OS: Windows
  • Browser Firefox 89.0.2 (64-bit)
  • Time zone: EST
@imwh0im
Copy link
Contributor

imwh0im commented Jul 1, 2021

In typescript?

imwh0im added a commit to imwh0im/dayjs that referenced this issue Jul 1, 2021
@imwh0im
Copy link
Contributor

imwh0im commented Jul 2, 2021

In Javascript

var dayjs = require("dayjs");

console.log(dayjs(null).format('lll z'));  // Invalid Date

In Typescript

import dayjs from 'dayjs'

console.log(dayjs(null).format('lll z'));

returned

error TS2769: No overload matches this call.

i tried reproduced this issue. but not reproduced.
but find returned Error in Typescript. so fix it.

give more details for this issue?

@SamuelPoulin
Copy link
Author

My bad, I should have mentionned that the z format option is only enabled with advancedFormat. Here's a working example of the issue:

index.js

const dayjs = require("dayjs");

const utc = require("dayjs/plugin/utc");
const timezone = require("dayjs/plugin/timezone");
const advancedFormat = require("dayjs/plugin/advancedFormat");

dayjs.extend(utc)
dayjs.extend(timezone)
// dayjs.extend(advancedFormat) // When this is enabled, dayjs throws. When it's not, it outputs 'Invalid Date'.

console.log(dayjs(null).format('lll z'))

@imwh0im
Copy link
Contributor

imwh0im commented Jul 5, 2021

i checked plugin advancedFormat.
https://github.com/iamkun/dayjs/blob/dev/src/plugin/advancedFormat/index.js#L12-L51

if you want use the original format(). Do not extend advancedFormat.
Because it changes the function of format() with the plugin advancedFormat.
if you extends advancedFormat. You must to follow the rules.

https://day.js.org/docs/en/plugin/advanced-format check this document.

Apart from this, it seems that the returned values ​​of moment and dayjs are different. so need fix that.
I have the same thoughts as you

@SamuelPoulin
Copy link
Author

I completely missed the section in the advancedFormat docs where it says it changes the format function of dayjs. Still, throwing rather than giving Invalid Date is a weird edge case that I'm sure I'm not the only one to catch on deployed code. 😁

I made sure my code does not give a null parameter to an advancedFormat constructor for now, thanks again for working on this

imwh0im added a commit to imwh0im/dayjs that referenced this issue Jul 6, 2021
- iamkun#1558
- add validation for invalid date in plugin advancedFormat
@imwh0im
Copy link
Contributor

imwh0im commented Jul 7, 2021

Merged complete

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