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

Cannot perform arithmetic on Dates #9356

Closed
bcherny opened this issue Jun 24, 2016 · 19 comments
Closed

Cannot perform arithmetic on Dates #9356

bcherny opened this issue Jun 24, 2016 · 19 comments
Labels
Duplicate An existing issue was already created

Comments

@bcherny
Copy link

bcherny commented Jun 24, 2016

new Date() - new Date()
// [ts] The right-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type.

JS will implicitly convert these to numbers via valueOf, so explicit conversion is not necessary.

@Arnavion
Copy link
Contributor

Duplicate of #8260

@bcherny
Copy link
Author

bcherny commented Jun 25, 2016

@Arnavion It doesn't seem fair to close #8260. Implicits for built in types are a key part of the ES1 spec, and should at some point be implemented by TS. It can be prioritized lower than other things, but it should not be ignored.

@Arnavion
Copy link
Contributor

It's not being ignored, it's omitted by design. Implicit conversions are the source of bugs. If all valid ES was valid TS there would not be much point to TS.

If one really wants to subtract dates then that issue shows how to convince the compiler to allow it.

@bcherny
Copy link
Author

bcherny commented Jun 25, 2016

Is there a design document, or something else that talks about this idea? Are there other parts of the ES spec that TS chose not to implement?

Implicits are understandably notorious for causing bugs in dynamically typed languages (as are almost all language features :)), but they are a core part of many statically typed languages (C#, Scala, Haskell), so it seems silly to omit them. Especially if you want to work with JS code in the wild, which uses toString, valueOf, and toJSON implicitly all the time.

@nippur72
Copy link

@bcherny the "design document" is TypeScript Design Goals which says:

  1. Statically identify constructs that are likely to be errors.

which I think applies the case of Date subtraction.

@bcherny
Copy link
Author

bcherny commented Jun 25, 2016

How do you plan on interoperating with existing, JSDoc'd code if the compiler doesn't understand implicits?

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jun 25, 2016

Consider

new Date() - new Date() // 0

That output is reasonable
But consider

var d = new Date();
d.setHours(3);
d - new Date() // -43218751
d + new Date() // "Sat Jun 25 2016 03:51:22 GMT-0400 (Eastern Daylight Time)Sat Jun 25 2016 17:54:37 GMT-0400 (Eastern Daylight Time)"

This is really weird behavior and I don't think it should be encouraged.

@bcherny
Copy link
Author

bcherny commented Jun 27, 2016

@aluanhaddad Encouraging/discouraging behavior should be left to tslint, I think.

TypeScript should first and foremost understand the rules of the ES specs, and help TS and existing JS code scale given those rules. If there's a whole bunch of existing JS code that TS is unable to infer and flow types for, that makes TS less valuable to a lot of developers!

@aluanhaddad
Copy link
Contributor

@bcherny in principle I absolutely agree with that statement. I oppose proposals that would fundamentally change the semantics of JavaScript and proposals which did not map intuitively to JavaScript.
However, JavaScript places no restrictions on the subset of values to which arithmetic operators can be applied. It's perfectly valid to add and subtract arbitrary objects, but these operations, while having well-defined semantics, are often absurd. In this case the subtraction of two values results in a number, but their addition results in a string. That doesn't seem to make a whole lot of sense and forms a subtle trap when changing the order of operations.

@RyanCavanaugh
Copy link
Member

@bcherny nearly everything TypeScript flags as an error has some valid runtime interpretation. If you want to use [ ] * { } as a way to produce NaN, well, that's according to spec, so TypeScript shouldn't disallow it?

This is really a dupe of #2361 because Date implements valueOf that sometimes returns number.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Jun 27, 2016
@aluanhaddad
Copy link
Contributor

@RyanCavanaugh thank you for calling date insane, that's really what I was trying to say, only you did so much more concisely ❤️

@bcherny
Copy link
Author

bcherny commented Jun 27, 2016

@aluanhaddad I agree that JS implicits can lead to subtle bugs!

@RyanCavanaugh There's an argument to be made that if someone writes:

let a: number = [ ] * { }

The TS compiler should be OK with it, since this is valid JS, and TS is a superset of JS. If someone then tries to do something with a assuming that it's, say, an array, the TS compiler should warn that "hey! this is actually a number!". If you consider this bad style, then create a tslint rule for it for your project.

Anyway, to wrap up it sound like what you guys are saying is "TS is ok with some implicits (eg. conversion to boolean via logical operators like ! or &&), but not others (eg. conversion to number via arithmetic operators like + or -), because the former are used idiomatically in JS, while the latter are mostly a source of bugs".

@bcherny bcherny closed this as completed Jun 27, 2016
@RyanCavanaugh
Copy link
Member

That's basically correct. The unary + / - operators are relatively hard to use incorrectly.

The real problem with Date is that its implicit conversion rules are just nuts. x - 0 is a number, x + 0 is a string, and (x + 0) * 1 is NaN ? It's a loaded footgun.

image

@mihailik
Copy link
Contributor

Can this be reopened please?

Date arithmetic is a valid correct and reasonable operation completely within JS day-to-day established good practices. Without it things like max/min date, average date, closest date are a pain to write.

Are there practical use cases where preventing date arithmetic would highlight a genuine error? The examples above are extremely artificial.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Sep 13, 2016

@mihailik It seems like a - b should be equivalent to a + (-b) but it is not for Date. How is that an artificial concern. As soon as you abstract over your operands you end up with a likely source of bugs. For example, to get the average of a set of Dates one might reasonably write code like

const dates = [new Date(), new Date(), new Date(), new Date(), new Date()];

const average = dates.reduce((total, date) => total + date, 0) / dates.length;

This results in NaN.

@RyanCavanaugh
Copy link
Member

Date arithmetic fails the most basic of mathematical identities, e.g. x + 1 - 1 !== x - 1 + 1. I don't understand how this is "extremely artificial" when x * 2 !== x + x. It can't safely be reasoned about.

@mihailik
Copy link
Contributor

Artificial though it is, x + 1 - 1 === x - 1 + 1 doesn't safely hold for the good old Number either, but we put up with imperfections.

I guess what you're really trying to say is that in past you've been burnt by dates, and prefer to skip the whole business. I better stop insisting.

@icholy
Copy link

icholy commented Feb 3, 2017

I've started working on a solution to this over in #2361

@Roam-Cooper
Copy link

Potential solution could just be allowing a cast to number or string...
Ie still disallow

let a: number = date1 - date2;

But allow

let a: number = <number>date1 - <number>date2;

And allow

let a: string = <string>date1 + <number>date2;

But obviously disallow

let a: number = date1 + <number>date2;

Ie disallow the operation by changing what the return type of the arithmetic is considered to be, but allow casting dates in date arithmetic so that the return type can be perceived as string or as number.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

9 participants