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

5-digit years may fail in some environments #73

Open
diegodlh opened this issue Jul 16, 2021 · 0 comments
Open

5-digit years may fail in some environments #73

diegodlh opened this issue Jul 16, 2021 · 0 comments

Comments

@diegodlh
Copy link
Contributor

In PR #68 I commented

you write "Only trying to parse 5-digit years or below". Shouldn't 5-digit years fail? Shouldn't that be 4-digits only?

to which @maxlath replied

On the 5-digit year point: while most cases are limited to 4-digit years, Wikibase supports date far in the past and the future (example), so this lib should probably do what it can to support it

to which I replied

Sure! But the point I'm referring to is the check you make by converting the time to an ISO string. I understand 5-digit years would fail there (you're already skipping this check for years longer than 5 digits).

I think we were misunderstanding each other because of browser differences. Whereas new Date('12021') (a 5-digit year) fails in my Firefox 89 console, it doesn't fail in my Chrome 91 console. That is, whereas new Date seems to support 6-digit years up to 275760 in Chrome (see here), it seems to support years up to 9999 in Firefox (see here). MDN warns about these browser differences and inconsistencies, and discourages parsing of date strings with the Date constructor.

To fix this we could either:

  • only try parsing 4-digit years or below, or
  • check whether new Date(99999) works to decide whether a 4- or 5-digit year threshold should be used.

What do you 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

No branches or pull requests

1 participant