Skip to content

typescript passes number | Promise<number> as if it were number (or something like that) #52036

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

Closed
azizghuloum opened this issue Dec 28, 2022 · 10 comments · Fixed by #52048
Closed
Labels
Experimentation Needed Someone needs to try this out to see what happens Suggestion An idea for TypeScript

Comments

@azizghuloum
Copy link

Bug Report

Something is very weird that makes typescript pass Promise<number> as if it were a number when it is an object field. Typescript correctly recognizes that Promise<number> is not a number when it is a plain variable. Weird.

🔎 Search Terms

promise numeric

🕗 Version & Regression Information

I believe this has always existed in typescript. I have tried multiple versions on playground (from 3.3 up to 5). I did read the FAQs in https://github.com/Microsoft/TypeScript/wiki/FAQ#common-bugs-that-arent-bugs and found nothing related.

⏯ Playground Link

Playground link with relevant code

💻 Code

const n: number | Promise<number> = Promise.resolve(0);
console.log(n >= 0);  // Correctly rejected: Operator >= cannot be applied to Promise<number> and number.

const foo: {n: number | Promise<number>} = {
  n: Promise.resolve(0),
};
console.log(foo.n >= 0); // Incorrectly type checks.  Runtime result is bogus.

🙁 Actual behavior

The last expression passes the type checker even though it is clearly wrong.

🙂 Expected behavior

That the last expression is rejected with Operator >= cannot be applied to Promise<number> and number..

@azizghuloum
Copy link
Author

azizghuloum commented Dec 28, 2022

n and foo.n both have the exact same type here, but they are somehow treated differently when found in n >= 0 vs foo.n >= 0. Weird.

@fatcerberus
Copy link

The difference is that n is immediately narrowed on assignment to Promise<number>, while foo.n is not narrowed and remains typed as number | Promise<number>.

const n: number | Promise<number> = Promise.resolve(0) as number | Promise<number>;
console.log(n >= 0);  // typechecks, as with foo.n

@azizghuloum
Copy link
Author

azizghuloum commented Dec 28, 2022

Thanks for explaining the difference @fatcerberus.
(though it's also weird that it narrows despite my explicit type specification but let's not get distracted)

Now why number | Promise<number> typechecks is still the issue.

@azizghuloum azizghuloum changed the title typescript sometimes thinks promises are nonpromises (or something like that) typescript passes number | Promise<number> as if it were number (or something like that) Dec 28, 2022
@fatcerberus
Copy link

though it's also weird that it narrows despite my explicit type specification

People run into this a lot; long story short is that this is considered the lesser of two evils as opposed to the alternative. See for example #50444 (comment):

Initialization does perform narrowing. Not doing this opens you up to stuff that just looks broken as heck; this used to not happen and everyone hated it.


Now why number | Promise<number> typechecks is still the issue.

Yeah, agree 100%. That really shouldn't typecheck.

@Andarist
Copy link
Contributor

I've checked what happens under the hood and it seems that it's enough for left and right types to be comparable in either direction:
https://github.dev/microsoft/TypeScript/blob/c7f49bceed878d751701f018d066fffc5926e3b0/src/compiler/checker.ts#L35641

This is quite surprising but apparently it's even OK to compare class instances (test case taken from the TS test suite): TS playground

I spiked a potential improvement for the reported case here but I'm not totally sure if this is something that the TS team would like to incorporate.

@fatcerberus
Copy link

It seems weird that it’s using the comparability relation for that. That x == y is allowed doesn’t imply that x >= y should also be.

@azizghuloum
Copy link
Author

Yeah I guess EcmaScript allows pretty much anything to be on either side of a Relational Operator and the runtime semantics of that, when given mixed inputs gets unclear (to put it mildly).

Not sure where TypeScript stands. Does it wants to stick with the clear subset (comparing two numbers or two strings) or does it want to extend to ES territory?

@fatcerberus
Copy link

Technically, there is no undefined behavior in ECMAScript; everything is well-defined by the specification, so TypeScript almost by definition must be pragmatic about what it considers an error (e.g. it is not allowed to call parseInt() with a number, since its coercion behavior, while defined by ES, is often unintuitive and likely not to do what the developer expects it to). See https://stackoverflow.com/questions/41750390/what-does-all-legal-javascript-is-legal-typescript-mean for more discussion on that.

Applying relational operators to arbitrary objects definitely feels to me like something that is likely to be a coding mistake and should produce a TS error. The main question is whether that behavior can be changed now without causing disruptive breaking changes.

@RyanCavanaugh
Copy link
Member

I believe this was intentionally allowed at one point. Since object operands always return false in relational operators, I'd argue that it's at least somewhat coherent to do this. #41163 (comment)

We'll look at what the PR runs turn up; maybe no one actually does this on purpose.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experimentation Needed Someone needs to try this out to see what happens labels Jan 5, 2023
@RyanCavanaugh
Copy link
Member

I dug through the archives and found #10120 as the most canonical investigation of this, as well as #5156

It's a bit tricky since I think there's a reasonable argument that

function fn(x: number | (() => string)) {
  if (x > 10) {
  }
}

is every bit as valid of a type guard on x as typeof x === "number", because the coercion from (() => string) to number is (almost) guaranteed to produce NaN -> false, and so should be allowed (and also narrow, which it doesn't today!).

But the fact that we're not currently narrowing x here would seem to imply that no one is likely to be intentionally relying on this behavior. And code like this

function fn(x: number | string) {
  if (x > 10) {
  }
}

is clearly every bit as suspect as "3" * 4 in terms of its possible runtime semantics.

This kind of has the same flavor to me as the problem of boolean coercion we see elsewhere. There's code like

function fn(x: object | undefined) {
  if (x) { }
}

which seems very cromulent, and code like

function fn(x: number) {
  if (x) { }
}

which also seems very cromulent, but code like

function fn(x: number | undefined) {
  if (x) { }
}

seems like a giant red flag because it's not really clear if the author fully thought through the falsiness of 0 or not. You'd at least plausibly want this behavior to be configurable. But it's not obvious what the distinguishing principle in these examples is; the rule is something like "You can do a coercion as long as only one constituent of the union is nominally capable producing both true and false".

The same rule seems like what you want here - you can do x > y where x is number and y is T | U | V as long as exactly one of T | U | V predictably coerces to a finite value and the rest predictably coerce to NaN (under reasonable assumptions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experimentation Needed Someone needs to try this out to see what happens Suggestion An idea for TypeScript
Projects
None yet
4 participants