Skip to content

String.prototype.replace docs need fixing #47969

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

Closed
SamB opened this issue Feb 19, 2022 · 0 comments · Fixed by #50041
Closed

String.prototype.replace docs need fixing #47969

SamB opened this issue Feb 19, 2022 · 0 comments · Fixed by #50041
Assignees
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@SamB
Copy link

SamB commented Feb 19, 2022

lib Update Request

Configuration Check

My compilation target is es2020 and my lib is the default.

Missing / Incorrect Definition

The documentation for String.prototype.replace is confusing, and some of it is wrong.

Here are the definitions:

/**
* Replaces first match with string or all matches with RegExp.
* @param searchValue A string or RegExp search value.
* @param replaceValue A string containing the text to replace for match.
*/
replace(searchValue: { [Symbol.replace](string: string, replaceValue: string): string; }, replaceValue: string): string;
/**
* Replaces text in a string, using an object that supports replacement within a string.
* @param searchValue A object can search for and replace matches within a string.
* @param replacer A function that returns the replacement text.
*/
replace(searchValue: { [Symbol.replace](string: string, replacer: (substring: string, ...args: any[]) => string): string; }, replacer: (substring: string, ...args: any[]) => string): string;

TypeScript/src/lib/es5.d.ts

Lines 424 to 436 in 1ad569f

/**
* Replaces text in a string, using a regular expression or search string.
* @param searchValue A string to search for.
* @param replaceValue A string containing the text to replace for every successful match of searchValue in this string.
*/
replace(searchValue: string | RegExp, replaceValue: string): string;
/**
* Replaces text in a string, using a regular expression or search string.
* @param searchValue A string to search for.
* @param replacer A function that returns the replacement text.
*/
replace(searchValue: string | RegExp, replacer: (substring: string, ...args: any[]) => string): string;

The first overload in es2015.symbol.wellknown.d.ts just plain has the wrong text, and it's also confusing because it says "or all matches with RegExp".

The first overload in es5.d.ts uses the phrase "for every successful match of searchValue in this string", which is technically correct I guess, but fails to mention that the only (applicable) case in which more than one match is even attempted is when searchValue is a RegExp with the g flag set (or at any rate, a RegExp where the spec equivalent of searchValue.global is considered true).

Sample Code

This isn't about a failure to type-check, but a failure to explain.

Documentation Link

See https://tc39.es/ecma262/#sec-string.prototype.replace and https://tc39.es/ecma262/#sec-regexp.prototype-@@replace. (MDN isn't super clear on this particular method, either.)

N.B. there's PR in flight that touches the same code: #44348

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Fix Available A PR has been opened for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants