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

instant.valueOf could return epochNanoseconds instead of throwing #3080

Closed
mbostock opened this issue Jan 30, 2025 · 1 comment
Closed

instant.valueOf could return epochNanoseconds instead of throwing #3080

mbostock opened this issue Jan 30, 2025 · 1 comment

Comments

@mbostock
Copy link

mbostock commented Jan 30, 2025

If I follow the discussion in #1681 #517 correctly, along with the explained rationale, instant.valueOf was made to throw an error because the relational operators <, <=, >, or >= would not work correctly if the instant is implicitly coerced to a string. However, if you instead have instant.valueOf return the epochNanoseconds as a bigint, these operators would work just fine. So can we do that? This would also fix the use case I previously described in #1840.

Here’s a simple patch:

Temporal.Instant.prototype.valueOf = function() {
  return this.epochNanoseconds;
};

And two test cases:

const i1 = Temporal.PlainDate.from("2020-01-04").toZonedDateTime({timeZone: "America/Los_Angeles"}).toInstant();
const i2 = Temporal.PlainDate.from("2020-01-08").toZonedDateTime({timeZone: "America/Los_Angeles"}).toInstant();

i1 < i2;  // true
const i3 = Temporal.PlainDate.from("-002020-01-04").toZonedDateTime({timeZone: "America/Los_Angeles"}).toInstant();
const i4 = Temporal.PlainDate.from("-002222-01-04").toZonedDateTime({timeZone: "America/Los_Angeles"}).toInstant();

i3 < i4;  // false since 2020 BCE is after 2222 BCE

Furthermore, since the Temporal.Instant construct accepts a bigint, it makes sense that valueOf would return the same.

I see now this was previously suggested in js-temporal/proposal-temporal-v2#6 (comment); is it too late to change? (And I guess this argument applies to Temporal.ZonedDateTime, too.)

@justingrant
Copy link
Collaborator

I see now this was previously suggested in js-temporal/proposal-temporal-v2#6 (comment); is it too late to change? (And I guess this argument applies to Temporal.ZonedDateTime, too.)

Temporal is very close to being incorporated into the ECMAScript standard and has not been making changes (other than those demanded by JS engine implementers that were blocking implementation) for a few years. So the V2 repo is a good place for feedback like this.

When the decision was made many years ago that valueOf should throw, there was hope that operator overloading might be added into the language, which would provide a much better solution than converting to a primitive type. Sadly, operator overloading hasn't happened so a future proposal may be more open to adding a non-throwing valueOf.

So I'm going to close this issue and I encourage you to add your feedback over in the V2 repo. Thanks!

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