Skip to content

Replacement of standard String.prototype functions causes issue in other extensions #19761

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
Codeneos opened this issue Aug 31, 2022 · 2 comments
Assignees
Labels
triage-needed Needs assignment to the proper sub-team

Comments

@Codeneos
Copy link

Codeneos commented Aug 31, 2022

In extensions.ts the string prototype function replaceAll is replaced by a non-standard implementation. The new implementation in this extension does not comply with the standard signature of replaceAll which causes other extension to fail.

src/client/common/extensions.ts

/**
 * String.replaceAll implementation
 * Replaces all instances of a substring with a new substring.
 */
String.prototype.replaceAll = function (this: string, substr: string, newSubstr: string): string {
    if (!this) {
        return this;
    }

    /** Escaping function from the MDN web docs site
     * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping
     * Escapes all the following special characters in a string . * + ? ^ $ { } ( ) | \ \\ */

    function escapeRegExp(unescapedStr: string): string {
        return unescapedStr.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
    }

    return this.replace(new RegExp(escapeRegExp(substr), 'g'), newSubstr);
};

This will fail when the substr argument is a RegExp; but the etandard spec defined that replaceAll should accept a RegExp.

But apart from the correctness of the replacement no extension should replace a functions oon common prototypes such as string.

Would it be possible to revert this and instead use a custom extension function?

export function replaceAll(this: string, substr: string, newSubstr: string);

-or-

export function replaceSubstring(this: string, substr: string, newSubstr: string);
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Aug 31, 2022
@karrtikr
Copy link

Thanks for investigating the cause, this is a duplicate of #18871.

@karthiknadig
Copy link
Member

Closing in favor of #18871

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
triage-needed Needs assignment to the proper sub-team
Projects
None yet
Development

No branches or pull requests

3 participants