Make RegExpMatchArray index and input field non-optional#50211
Make RegExpMatchArray index and input field non-optional#50211TomasHubelbauer wants to merge 1 commit intomicrosoft:mainfrom TomasHubelbauer:patch-1
Conversation
On MDN, I don't see documentation for any case where either of these could be `undefined`. This change dates all the way back to 2017 when @jedmao asked @DanielRosenwasser about it, but there was no response: 7bf846a#diff-88c6092c5611bde0c0df8f538cb4334bcdc2860d73616190f1c3d3095ceda301R794 Either this is a mistake or there indeed is a case where either of these can be `undefined` but if that's the case, can anyone please educate me on that? I will update the MDN page to include information about this as well as add JSDoc description to the fields so that this information appears on hover, too. The fields being optional is problematic because either one has to add a condition that will never be useful for anything or use the `!` operator. The problem with the latter is that it is not obvious why it needs to be there (circling back to the root of this issue) and that TypeScript is also used for type checking JavaScript with JSDoc and JavaScript has no `!`, so code like this won't work in JavaScript checked with TypeScript: ```javascript for (const match of ''.matchAll(/something/g)) { results[match.index] = match[0]; } ``` In a case like this, the only solution is to use `@ts-ignore` and risk missing other erros on that line or to add the presumable useless condition for checking `index` is not `undefined`.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
#36788 is related. |
|
I think e.g. https://github.com/alexrecuenco/TypeScript/commit/95ddd79bb22d3db9e6da9c6c2f5527cc01cbdf78 is the right fix here instead? |
|
So a few things: First, we've outlined a few rules to make it easier to manage and facilitate discussions. We understand that they add more work for contributors, but they mean that the team can scale out better at managing issues, PRs, and engaging with the community. Making us take the time to say "no, please add tests as we've asked" and disregarding our issue templates is not constructive. Second, I probably did not see that comment - leaving comments on commits is just less typical for us and typically gets lost in the shuffle of the issue tracker. Apologies to @jedmao. Third - looking a little closely at the ECMAScript spec, I do think that there is a mismatch between what we have and what gets created in the runtime.
So // src/lib/es5.d.ts
interface RegExpMatchArray extends Array<string> {
// always present
index: string;
// always present
input: string;
// always present - helpful for '--noUncheckedIndexedAccess'
0: string;
}// src/lib/es2018.regexp.d.ts
interface RegExpMatchArray extends Array<string> {
// always present - helpful for '--exactOptionalPropertyTypes'
groups: { [key: string]: string } | undefined;
} |
|
Rereading the spec, the global flag ensures that they do not share the same code path, so I was wrong. https://tc39.es/ecma262/#sec-regexp.prototype-@@match @RyanCavanaugh found an easy example of when they have very different shapes. |
On MDN, I don't see documentation for any case where either of these could be
undefined. This change dates all the way back to 2017 when @jedmao asked @DanielRosenwasser about it, but there was no response:7bf846a#diff-88c6092c5611bde0c0df8f538cb4334bcdc2860d73616190f1c3d3095ceda301R794
Either this is a mistake or there indeed is a case where either of these can be
undefinedbut if that's the case, can anyone please educate me on that? I will update the MDN page to include information about this as well as add JSDoc description to the fields so that this information appears on hover, too.The fields being optional is problematic because either one has to add a condition that will never be useful for anything or use the
!operator. The problem with the latter is that it is not obvious why it needs to be there (circling back to the root of this issue) and that TypeScript is also used for type checking JavaScript with JSDoc and JavaScript has no!, so code like this won't work in JavaScript checked with TypeScript:In a case like this, the only solution is to use
@ts-ignoreand risk missing other erros on that line or to add the presumable useless condition for checkingindexis notundefined.Backlogmilestone (required)mainbranchgulp runtestslocally