Throwing in valueOf causes errors when date is casting to string

[revyh]

The issue is that expressions like '' + Temporal.now.instant() throw error. I've read through several discussions about valueOf (namely #517 and #74) and I think I understand the motivation behind the decision to throw errors from valueOf. But at the same time, throwing an error in such a widespread way of casting to string makes Temporal dates very fragile.

Of course, you can workaround this by using something like Temporal.now.instant().toString() or `${Temporal.now.instant()}`. But the real problem arises when you want to use Temporal in a code that you don't control. For example, I encountered this problem when I tried to use Temporal as a prop in a Vue component. I assume the same thing can happen with other libraries, especially the older ones. Or even worse, transpilers or minifiers can assume that it's safe to transform `Current date is ${Temporal.now.instant()}` to 'Current date is ' + Temporal.now.instant() or Temporal.now.instant().toString() to ''+Temporal.now.instant(). So, your code may work fine in development but break in production.

[LinusU]

[...] Or even worse, transpilers or minifiers can assume that it's safe to transform `Current date is ${Temporal.now.instant()}` to 'Current date is ' + Temporal.now.instant() or Temporal.now.instant().toString() to ''+Temporal.now.instant(). So, your code may work fine in development but break in production.

I don't think that any transpiler or minifier does this, and if they do then it seems like a bug to me. Consider the following code that would also break then:

const foo = {
  valueOf: () => '1',
  toString: () => '2'
}

console.log('' + foo)
console.log(`${foo}`)

e.g. Babel correctly transpiles this into:

"use strict";

var foo = {
  valueOf: function valueOf() {
    return '1';
  },
  toString: function toString() {
    return '2';
  }
};
console.log('' + foo);
console.log("".concat(foo));

[revyh]

Alright, maybe I'm wrong about transpilers and minifiers (I hope so), but this doesn't solve problems with libraries

[ljharb]

It’s incorrect to concat to a string to convert to one, for this exact reason (Date objects are affected too). Code should be using String(x) or `${x}` to convert to a string.

[ptomato]

@revyh Yes, you're right that libraries may do this. We've seen one instance of this so far with React, #1224 / facebook/react#20594 which was fixed quite promptly. Is it possible to isolate the place in Vue where this is happening and report a bug?

[revyh]

@ljharb yes, now I understand that. But just recently I thought that concatenation with empty string is just another way to cast something to string and that it's absolutely safe to sum anything with a string. And I assume that many people think the same way despite the fact that it's a misunderstanding. And these people authoring libraries. And by breaking this assumption we potentially break a large number of libraries.

@ptomato the issue with vue relates to the code that validates passed props according to propTypes. And I can overcome this issue simply by using more specific propTypes like type: Temporal.Instant or type: [Temporal.Instant, Temporal.ZonedDateTime] (initially it was just type: Object which caused errors). Also, I experienced this with vue v2.6 and I don't know if this issue persists in v3. Considering that vue2 is in maintenance mode I don't think that it's reasonable to report this bug. But I'll report more serious bugs to the vue repo if I encounter them.

I guess, that we've seen this problem with only few libraries so far because the Temporal API is not widely used currently. But with the growing adoption of the Temporal API we'll see more bugs like this.

As I see, you guys are well aware of these consequences and your decision is conscious. So, feel free to close the issue.

Just in case, if someone thought that I'm offended by the unwillingness to make changes then it's not the case. I understand that it was a tough decision and it's unlikely that someone wants to reiterate on this, especially at stage 3. I'm just trying to make the conversation productive.

[ljharb]

@revyh such libraries were already broken by Date objects, since the dawn of JavaScript, so I think it's reasonable to file issues rather than to worry about "the replacement for Date objects" triggering the same bugs as "Date objects" do.

[bakkot]

It's debatable whether they were broken by Date - they'd still do something other than throwing - but they were certainly broken by the introduction Symbol, which has the same throwing behavior as here.

[ptomato]

I think we should close this, but keep an eye on whether more of these bugs pop up in libraries, especially if they can't be fixed.

(However, I'm not sure we'd change the fact that valueOf() throws, even if they did; it's more likely we'd look for a different mitigation. Having code like plaindate1 < plaindate2 (which accidentally does lexicographical comparison) usually-but-not-always give the correct answer, seems much more damaging than these kinds of bugs)