ts2365 allows unsafe comparisons: string | number < string | number

[cben]

🔎 Search Terms

ts2365
relational comparison
binary operator <
binary operator >
binary operator <=
binary operator >=
TypeScript 5

🕗 Version & Regression Information

• v4.9.5 gave zero errors for this code.
• TS 5.0 added errors for lt1 and lt2 which I subjectively like — they promote more explicit code 👍 — but are actually safe type combinations (Improve comparison operators type checking to disallow unions containing numbers as an operand #52048, ann).
• however lt3, lt4 and lt5 are dangerous at runtime yet allowed by TS.

⏯ Playground Link

link — Run and see Logs

💻 Code

const show = (a: any, ltResult: boolean, b: any) =>
    `${JSON.stringify(a)}${ltResult ? ' < ' : ' >='}${JSON.stringify(b)}\t`

// These two show TS2365 error on '<' operators (TS 5.2.2),
// but the allowed arg combinations actually always compared numerically:
const lt1 = (a: string | number, b: number         ) => show(a, a < b, b)
const lt2 = (a: number         , b: string | number) => show(a, a < b, b)

// These three TS 5.2.2 does NOT complain, but are unsafe!
// They allow string<string which compares lexicographically AND 
// one string one number which which compare numerically
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Less_than#description
const lt3 = (a: string | number, b: string         ) => show(a, a < b, b)
const lt4 = (a: string         , b: string | number) => show(a, a < b, b)
const lt5 = (a: string | number, b: number | string) => show(a, a < b, b)

// Only including calls allowed by argument types:

console.log(`lt1: ${lt1(12, 9)} ${lt1('12', 9)}`)
console.log(`lt2: ${lt2(12, 9)}                 ${lt2(12, '9')}`)
// lt3-5 are 100% TS-clean but mix true/false results depending on run-time types!
console.log(`lt3:                             ${lt3(12, '9')} ${lt3('12', '9')}`)
console.log(`lt4:             ${lt4('12', 9)}                 ${lt4('12', '9')}`)
console.log(`lt5: ${lt5(12, 9)} ${lt5('12', 9)} ${lt5(12, '9')} ${lt5('12', '9')}`)

🙁 Actual behavior

lt1–2 give errors Operator '<' cannot be applied to types 'string | number' and 'number'. and vice versa.
string vs. number comparisons actually convert both sides to number so these consistently compare numerically — Run ⏯️ playground and you'll see Logs consistently say 12 >= 9 here.

lt3-5 examples OTOH give no TS errors but allow a string < string case!
Only when both sides are strings (or convert to primitive as strings, despite number hint), JS does lexicographic string comparison.
You'll see in ⏯️ Logs these return mix of < and >= results, depending on whether they were both strings or at least one number at run-time 💥

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Less_than#description, https://262.ecma-international.org/#sec-islessthan

🙂 Expected behavior

lt3: Operator '<' cannot be applied to types 'string | number' and 'string'.
lt4: Operator '<' cannot be applied to types 'string' and 'string | number'.
lt5: Operator '<' cannot be applied to types 'string | number' and 'string | number'.

Additional information about the issue

I suppose the lt5 case is the most "interesting", because the types on both sides are same, and from type-system perspective they're valid sub-set of types < can handle.
In particular this is what you'd write to allow string < string AND number < number at run time, both being perfectly legit combinations to execute.
BUT lexicographic vs. numeric comparisons are semantically different operations (that just happen to share same operator), and practically always you only want one of these meanings when writing the code.

(cf. #49661 which asked about val + val. The answer there was it's about types not identity, but I'd say the more fundamental answer is you meant either addition OR concatenation.)

I don't know what to say about wider types e.g. any < any 🤷 Same argument applies that you only meant one or the other meaning, but enforcing that would break TS goal of gradual typing.

cc @Andarist @RyanCavanaugh

[cben]

• Real-app story which prompted this issue:
  We have form fields where user types numbers, and we mixed string/number representations sometimes for same field 🤦‍♂️ (JS codebase gradually converting to TS...)
  So for now we have lots of validations taking string | number. Most do e.g. value >= 1 which TS 5 started to warn about but are actually safe — OTOH we have a min_field <= max_field validation which may be really risky and could benefit from error!
  (well tbh min_field and max_field are now typed any because validator takes allValues: object and per-field typing with redux-form and Formik is more laborious, a "higher-hanging fruit" we hadn't bothered yet; we're likely to tighten our NodesInput component to store a single known type, before we get around to typed allValues... But imagine a smaller app with validation near concete fields.)

[MartinJohns]

I suppose the lt5 case is the most "interesting", because the types on both sides are same, and from type-system perspective they're valid sub-set of types < can handle.

Left argument can be string and right one number.

[cben]

yeah, my passage about lt5 being "interesting" was a strawman attack on myself playing devil's advocate... You're right, "types on both sides are same" was meaningful hand-waving.

My POV is the risky behavior worth discouraging is not so much mixing string < number — those are well-defined though bad style as one has to look up what they'll do — but types that may resolve to both lexicographic or numeric comparison at run time.

• Valid use case where lt5 is legit:
  A universal sorting function taking any[] or string[] | number[] or things like that can be sanely defined to use "natural JS < sorting order": an array of numbers to be sorted numerically, an array of strings lexicographically, and the choice will be known at run time.
  Such a function will do an lt5-like comparison somewhere!

  IMHO this is rare enough to require users to stop, think, and add ts-ignore?

[RyanCavanaugh]

Fix attempted at #52807 from prior discussion