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

Test ISO strings with extended year -000000 #3470

Merged
merged 1 commit into from
Apr 25, 2022

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Apr 9, 2022

In Date.parse() and new Date(), representations of the year 0 as -000000
must not be accepted. In the case of Date.parse(), they should yield NaN,
and in the case of new Date(), they should yield an invalid Date object,
whose valueOf() is NaN.

See tc39/ecma262#2550


for (const str of invalidStrings) {
assert(Number.isNaN(Date.parse(str)), "reject minus zero as extended year");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to block on this, but why not just write it like this?

assert.sameValue(Date.parse("-000000-03-31T00:45Z"), NaN);
assert.sameValue(Date.parse("-000000-03-31T01:45"), NaN);
assert.sameValue(Date.parse("-000000-03-31T01:45:00+01:00"), NaN);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right that sameValue does work on NaN. Just habits from elsewhere 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do prefer the loop since IMO it makes clear to the reader what changes between each assertion and what doesn't.

In Date.parse() and new Date(), representations of the year 0 as -000000
must not be accepted. In the case of Date.parse(), they should yield NaN,
and in the case of new Date(), they should yield an invalid Date object,
whose valueOf() is NaN.
@rwaldron rwaldron merged commit 8a1ec2f into tc39:main Apr 25, 2022
@ptomato ptomato deleted the extended-year-zero branch October 10, 2022 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants