microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.24k stars 12.39k forks source link

Add lacking documentation for Date constructor interface #45519

Open bruno-brant opened 3 years ago

bruno-brant commented 3 years ago

lib Update Request

Configuration Check

My compilation target is ES5 and my lib is the default.

Missing / Incorrect Definition

None - this is just improving documentation (jsdocs).

Sample Code

None - this is just improving documentation (jsdocs).

Documentation Link

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date

MartinJohns commented 3 years ago

this is just improving documentation (jsdocs).

You're actually introducing a breaking change in your PR... This is not just improving documentation.

bruno-brant commented 3 years ago

this is just improving documentation (jsdocs).

You're actually introducing a breaking change in your PR... This is not just improving documentation.

What change? Care to point me to it?

MartinJohns commented 3 years ago

The original version has a constructor overload accepting a number | string type. Your changes remove this overload and replaces it with two distinct overloads. This is a breaking change.

bruno-brant commented 3 years ago

Heck, you're completely right... I was under the impression that Typescript would consider it to be the same 🤦‍♂️. I'll fix it right now.

bruno-brant commented 3 years ago

@MartinJohns if can spare the time, could you explain why does this change is breaking? I've seem that the tests are failing, but I can't to come up with a sample where the code would break by splitting those methods.

MartinJohns commented 3 years ago

You can't call the constructor anymore with a variable typed as number | string:

const value = 0 as number | string;
new Date(value)
RyanCavanaugh commented 3 years ago

It'd be nice to iterate on these as individual issues, then get a PR to update the text once we've agreed on the phrasing.

bruno-brant commented 3 years ago

hey @RyanCavanaugh, I don't think the text is great either - I've copied it from MDN though, which seems to me to be an "official enough" source. They are, at the very least, correct.

What do you think: I believe the docs I proposed are better than nothing, so what do you say we approve it as it is and create an issue for improving it later? This way, we get docs asap, and then can work on it with a bit more time.

bruno-brant commented 3 years ago

And while we're at it, is there a guide for this kind of documentation? I wouldn't mind making those improvements and if there's some guidance I'd be happy to follow it.