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

String.prototype.split() with a regex containing a capturing group can return an array of string | undefined. #60941

Open
loristhesnail opened this issue Jan 9, 2025 · 5 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@loristhesnail
Copy link

⚙ Compilation target

any

⚙ Library

esnext, dom

Missing / Incorrect Definition

Using split with a negative lookahead and a capturing group can return undefined. See the attached screenshot. When iterating over the array, we lose type safety and encounter runtime errors.

Image

Sample Code

'a,b,c'.split(/,(?!(a))/) // Types as string[]

Documentation Link

No response

@MartinJohns
Copy link
Contributor

MartinJohns commented Jan 9, 2025

You need to enable noUncheckedIndexedAccess, that's what it is for.

Essentially a duplicate of #52299 and others.

@loristhesnail
Copy link
Author

loristhesnail commented Jan 9, 2025

Not really, it is a different issue.... I searched these before.

This should thrown a type error.

'a,b,c'.split(/,(?!(a))/).forEach((v) => {
  console.log(v.includes('something'))
})

@nmain
Copy link

nmain commented Jan 9, 2025

noUncheckedIndexedAccess won't change the types on forEach, map, for .. of, etc because they're not access from an index signature.

@jcalz
Copy link
Contributor

jcalz commented Jan 9, 2025

Yeah, this is a different issue.

There's a type safety hazard here for sure, given that the resulting array literally contains undefined, so --noUncheckedIndexedAccess doesn't help (if it were a sparse array then it would be weird but fine). But this is really an edge case, isn't it? TS wants to be type safe but not at the expense of productivity. In real world code, what percentage of uses of split() run into this? I'm guessing it's below 1%. That would mean for 99% of the code out there that uses split() today and has no problem, suddenly there's a useless compiler error they have to deal with. That would annoy everyone, and then people would just learn to ignore it (e.g., v!.includes('something')) , so if they do ever hit the real problem, they'd just hit it at runtime also. Can someone make an argument that the cure is better than the disease?

@nmain
Copy link

nmain commented Jan 9, 2025

If #32098 was implemented, that would lead towards a more precise solution here: The RegExp type would be generic in whether it had capture groups, and regex literals would automatically have the right generic parameters filled in for their inferred type. That could then infect the typings of things like RegExp.protoype[Symbol.split] and from there String.prototype.split.

Seems like a lot of work though.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants