fix first match in RegExpMatchArray being possibly undefined when noUncheckedIndexedAccess is enabled
#49682
Conversation
…noUncheckedIndexedAccess` is enabled
typescript-bot
added
the
For Uncommitted Bug
|
The TypeScript team hasn't accepted the linked issue #42296. If you can get it accepted, this PR will have a better chance of being reviewed. |
There was a problem hiding this comment.
This looks correct, but there are almost certainly people relying on RegExpMatchArray to be assignable to string[] which will be broken like assignFromStringInterface is. I want to estimate how big the break will be by looking at user tests and Definitely Typed tests.
| match(regexp: string): RegExpMatchArray; | ||
| match(regexp: RegExp): RegExpMatchArray; |
There was a problem hiding this comment.
why did these change? (and in stringLiteralTypeIsSubtypeOfString). Is it because RegExpMatchArray was formerly assignable to string[], but no longer is?
(or maybe the other way round, I never get variance the right way round the first time.)
There was a problem hiding this comment.
i think it's the other way around, that string[] used to be assignable to RegExpMatchArray but no longer is because of:
tests/cases/conformance/types/primitives/string/assignFromStringInterface2.ts(42,1): error TS2322: Type 'NotString' is not assignable to type 'String'.
The types returned by 'match(...)' are incompatible between these types.
Property '0' is missing in type 'string[]' but required in type 'RegExpMatchArray'.
i assume that since there were no new errors in DefinitelyTyped that this shouldn't be an issue?
|
@typescript-bot run dt |
|
Heya @sandersn, I've started to run the diff-based user code test suite on this PR at 93e0cfe. You can monitor the build here. Update: The results are in! |
|
@sandersn |
Fixes microsoft#49635 Behavior similar to microsoft#49682 microsoft#49635 was originally rejected because JavaScript has a stupid edge case where `''.split('')` and `''.split(new RegExp(''))` return an empty array so we cannot assert that the first element is *always* a string. However, given that the vast majority of use cases for `split` include a literal separator, we can create a set of overloads that results in the non-empty literal separator returning a more narrow type that has a string for the first element. ```ts ''.split('') // []; type= string[] ''.split(new RegExp('')) // []; type= string[] ''.split('a') // ['']; type= [string, ...string[]] ''.split(new RegExp('a')) // ['']; type= string[] ``` Unfortunately, I don't know of a way to differentiate a regular expression literal and a regular expression so that case still always returns an empty first element even though I don't even think it is possible to have an empty regular expression literal. I don't *believe* there are any other edge cases where split can return an empty array, but if there are please let me know and I can try to update, or we can close the PR and just leave a comment on microsoft#49635 for the next person.
Fixes #42296