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

Off-by-one error in blog dates #4881

Closed
ysulyma opened this issue Jun 1, 2021 · 10 comments · Fixed by #4983
Closed

Off-by-one error in blog dates #4881

ysulyma opened this issue Jun 1, 2021 · 10 comments · Fixed by #4983
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@ysulyma
Copy link
Contributor

ysulyma commented Jun 1, 2021

🐛 Bug Report

Dates in the blog section are displayed incorrectly. For example, if I create a file 2021-05-10-hello.md, the url will correctly be /blog/2021/05/10/hello, but the blog post displays "May 9, 2021".

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

I was unable to reproduce on https://new.docusaurus.io. However, I was able to trace the bug to https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-plugin-content-blog/src/blogUtils.ts#L175.

On my machine (both in Node and in the browser) new Date() behaves as follows:

new Date("2021-02-01").toString()
// "Sun Jan 31 2021 19:00:00 GMT-0500 (Eastern Standard Time)"

I was able to fix the bug on my machine by changing the above line to

date = new Date(dateString + "T00:00:00");

I don't want to make a PR myself because I'm not familiar enough with the Docusaurus code (or the Javascript Date functions) to know if this would fix it for all users or introduce bugs elsewhere.

Expected behavior

Dates would be displayed correctly.

Actual Behavior

Dates are not displayed correctly.

Your Environment

  • Public source code:
  • Public site url: https://ractive-player.org/blog/
  • Docusaurus version used: 2.0.0-beta.0
  • Environment name and version: Node 16.1.0
  • Operating system and version (desktop or mobile): Ubuntu 20.04.02 LTS ; also encountered this on OS X

Reproducible Demo

See above.

@ysulyma ysulyma added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jun 1, 2021
@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Jun 1, 2021

In my case it gets more frustrating, since 2021-05-10-hello.md actually creates a URL /blog/2021/05/09/hello during development, but the problem disappears in production. It's probably because I'm in GMT+0800...

@josh-kaplan
Copy link

I'm also seeing the same behavior and dove into the bug a bit more, @ysulyma's fix is close, but I think it needs to be a little further down on https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-plugin-content-blog/src/blogUtils.ts#L189.

@ysulyma , I found the same thing you did, but was using console.log to print the date object which always converts to the local time zone. I think the actual issue is below on line 189 where Intl.DateTimeFormat is called. Specifically, the timezone should be specified as UTC since the new Date(...) statements above automatically assume UTC. See below:

const formattedDate = new Intl.DateTimeFormat(i18n.currentLocale, {
            day: 'numeric',
            month: 'long',
            year: 'numeric',
            timeZone: 'UTC',
        }).format(date);

I'll review the Docusaurus contributing guidelines and if this is still open in a few days I'll see about opening a PR. In the meantime, I hope this helps.

@Josh-Cena
Copy link
Collaborator

@josh-kaplan I think you are on the right track. Are you going to open a PR? Docusaurus is pretty welcome to contributions.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Jun 16, 2021

The behavior of Date with time zones is just entirely cryptic to me...

image

I was able to locate my bug of URL offset to https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-plugin-content-blog/src/blogUtils.ts#L53. Firstly, my file name is like 2021-5-10-hello.md instead of 2021-05-10. According to the spec, the latter will be parsed as UTC but the previous will use the local time. And then, because toISOString transforms the date to UTC, you would have:

let d = new Date('2021-4-20') // "Tue Apr 20 2021 00:00:00 GMT+0800 (China Standard Time)"
d.toISOString() // "2021-04-19T16:00:00.000Z"

I think the easiest fix is to always set the time zone to UTC in creation of the date object rather than in transforming it to strings.

@josh-kaplan
Copy link

josh-kaplan commented Jun 16, 2021

That's interesting .... so it looks like new Date() either assumes UTC or local time depending on the format of your date string. It looks like the Date constructor uses 08:00 (note, I'm testing this in US/Eastern which is UTC-4) as the default time when the date is of the form YYYY-M-DD (or any similar formate where month or day is 1 digit), but uses 00:00 as the default time when the date is of the form YYYY-MM-DD.

new Date('2021-06-1')   // 2021-06-01T04:00:00.000Z
new Date('2021-6-01')   // 2021-06-01T04:00:00.000Z
new Date('2021-06-01')  //2021-06-01T00:00:00.000Z

I think you're right, @Josh-Cena, that UTC should be explicitly specified both when the date is created and when it is converted into a usable string so that Docusaurus respects that actual date string the author of the file intended. The only other concern still is whether or not this is still an issue for blog URL strings (I haven't tried it yet)

Interesting problem. I am curious why the timestamp changes for the different date formats...the MDN documentation isn't super clear about that.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Jun 16, 2021

It looks like the Date constructor uses 08:00 (note, I'm testing this in US/Eastern which is UTC-4) as the default time when the date is of the form YYYY-M-DD (or any similar formate where month or day is 1 digit), but uses 00:00 as the default time when the date is of the form YYYY-MM-DD.

Yes, I was totally bewildered at first... In fact, if you look at the "Timestamp strings" section in the MDN doc, you see the note that

Note: Parsing of date strings with the Date constructor (and Date.parse(), which works the same way) is strongly discouraged due to browser differences and inconsistencies.

Support for RFC 2822 format strings is by convention only.
Support for ISO 8601 formats differs in that date-only strings (e.g. "1970-01-01") are treated as UTC, not local.

Where ISO 8601 demands YYYY-MM-DD format. This is the only case where the string is treated as UTC rather than local.

In my PR I have enforced UTC whenever we are dealing with dates (the toUrl already uses toISOString, which is UTC). This should stop us from worrying about any time zone conversions.

@josh-kaplan
Copy link

I agree with your fix. I'm still baffled by the implicit 08:00 UTC timestamp (16:00 for you, 04:00 for me). But adding the "Z" to the date creation does seem to fix the issue.

@Josh-Cena
Copy link
Collaborator

Hmmm. Can you elaborate on the 08:00 thing? As I understand it, when the date object is created in the local timezone, the default timestamp is always 00:00, just that you later convert it to UTC.

2021-05-11T00:00:00 GMT+0800 => 2021-05-10T16:00:00 GMT+0000
2021-05-11T00:00:00 GMT-0400 => 2021-05-11T04:00:00 GMT+0000

@josh-kaplan
Copy link

Yeah, I'm not sure it impacts your fix though so if this becomes too unrelated to the bug, perhaps we should move the conversation elsewhere (it might still be relevant though) ...

When I do the following:

new Date('2021-06-01') // 2021-06-01T00:00:00.000Z
new Date('2021-06-1')   // 2021-06-01T04:00:00.000Z

Node defaults to different times for the two date formats (note they are both in UTC). In your case of UTC+0800, It looked like the time was defaulting to 16:00:00.000Z. Which seems to me like it's sort of caring about your offset from UTC (i.e. UTC+0800 = 1600 and UTC-0400 = 0400, which explains the different timestamps we're seeing), but then assuming the date is a UTC timestamp and resulting in 2021-06-01T04:00:00.000Z for me and 2021-06-01T16:00:00.000Z for you.

From the ECMA Spec

"The function first attempts to parse the String according to the format described in Date Time String Format (20.4.1.15), including expanded years. If the String does not conform to that format the function may fall back to any implementation-specific heuristics or implementation-specific date formats."

So, if you use the date format YYYY-MM-DD, the default time will be 00:00:00.000. But if you don't (i.e. your month or day is one digit instead of two), the Date constructor may fall back to "*any implementation-specific" approach. In other words, it's dependent on the JS implementation. I'm seeing the same behavior in both Node and in browser, presumably because they're both using V8.

I guess the real takeaway here is that your blog filenames should always use the YYYY-MM-DD format otherwise your local timezone could mess things up.

@slorber
Copy link
Collaborator

slorber commented Jun 16, 2021

Hey,

I agree that we should use both UTC for parsing and formatting, as we don't care about the time.

We could also use a lib that supports the concept of "local date", which is a given day without a timestamp/timezone.
https://js-joda.github.io/js-joda/manual/LocalDate.html

Probably overkill just to fix this bug though.

To test this it's a bit annoying/complicated as we'd have to force a certain tz on our CI, and we would have tests that only fail at certain hours of the day. Not sure it's worth investing much in that for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants