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

Operator '<' cannot be applied to types 'Number' and 'number' #52753

Closed
DarrenDanielDay opened this issue Feb 14, 2023 · 9 comments
Closed

Operator '<' cannot be applied to types 'Number' and 'number' #52753

DarrenDanielDay opened this issue Feb 14, 2023 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@DarrenDanielDay
Copy link

Bug Report

🔎 Search Terms

Number object
number primitive
binary operator <
ts2365
TypeScript 5.0-dev

🕗 Version & Regression Information

  • This changed between versions 4.9.5 and 5.0.0-dev.20230210

⏯ Playground Link

Playground link with relevant code

💻 Code

const a = new Number(1);
const b = 2;
console.log(a < b)

🙁 Actual behavior

In TypeScript 5.0.0-dev.20230210, it reports error ts2365, but in TypeScript 4.9.5, it doesn't.

🙂 Expected behavior

I'm not sure whether this is a bug of 5.0-dev or a breaking change that which will be introduced in 5.0. If it's a breaking change by design, maybe it should be included in #51362? If not, it should not report any error since it's valid JavaScript code.

Actually, I run into this error when I try to extend Number.prototype for utility:

interface Number {
    [Symbol.iterator](): Generator<number>;
}

Number.prototype[Symbol.iterator] = function*() {
    for (let i = 0; i < this; i++) yield i;
    //              ^^^^^^^^ ts2365
}

for (const i of 10) console.log(i);
@MartinJohns
Copy link
Contributor

MartinJohns commented Feb 14, 2023

This is mentioned in the release notes:
Breaking Changes: Forbidden Implicit Coercions in Relational Operators

You can fix this by adding an explicit coercion: Number(this)

@Andarist
Copy link
Contributor

Andarist commented Feb 14, 2023

Actually, I run into this error when I try to extend Number.prototype for utility:

This code is cursed 🤣

This is mentioned in the release notes:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0-beta/#forbidden-implicit-coercions-in-relational-operators

Yeah, it is - but this isn't about comparing a number to a non-number or about comparing a number to a union containing a number and other stuff. It is actually comparing a number to an instance of a Number class. This probably should be supported 🤔

@MartinJohns
Copy link
Contributor

This code is cursed 🤣

Absolutely agreed. I'd definitely smack whoever submitted code like this in a code review.

It is actually comparing a number to an instance of a Number class. This probably should be supported 🤔

I guess. We'll see what the team has to say. I'd argue it's a rare corner case and technically it is coercing to number.

@DarrenDanielDay
Copy link
Author

You can fix this by adding an explicit coercion: Number(this)

Yes, it works well, but the implicit conversion seems to be harmless, since it's just a comparison between Number and number. I think TypeScript should just report error about the dangerous implicit conversions like string to number, rather than safe conversions.

@Andarist
Copy link
Contributor

The Number case was even caught in my PR (see a comment here) but @RyanCavanaugh didn't respond with a yes/no answer if that should be corrected back then. And then the PR just landed :D

@DarrenDanielDay
Copy link
Author

This code is cursed 🤣

Absolutely agreed. I'd definitely smack whoever submitted code like this in a code review.

Absolutely I will also not submit such code in teamwork🤣. It's just utility code for leaning purpose, to show how to use prototype, generator function and iterator protocol in JavaScript.

On the other hand, is the type Number/ Number object impractical in teamwork? If so, it's reasonable to ignore the corner case in that PR.
Here's a relevant issue I've found, and it was opened years ago: #2361. I think it's time to reach a clear answer for it in 5.0. I'd see how the team would say.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 14, 2023
@RyanCavanaugh
Copy link
Member

#2361 is in a tough spot because we'd really like it to happen, but Date just completely breaks any attempt at unifying the codepaths due to its carve-out for hint behavior

@Josh-Cena
Copy link
Contributor

@DarrenDanielDay You should definitely coerce this to a number before iterating. Consider this:

Number.prototype[Symbol.iterator] = function* () {
  for (let i = 0; i < this; i++) yield i;
}

const v = Number.prototype.valueOf;
Number.prototype.valueOf = function () {
  console.log("coerced");
  return v.apply(this, arguments);
}

for (const i of 10) console.log(i);
// Logs "coerced" 10 times!

This is consistent with how built-in methods normalize input—always coerce at the boundary to minimize number of calls to user code.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

6 participants