Issue with "String.prototype.replaceAll()": need Polyfill + Suggestion

[PAPIONbit]

MDN URL: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll

Incomplete

I think the page need a polyfill, a good example:

if (!String.prototype.replaceAll) {
    String.prototype.replaceAll=function(search,replace) {
        return this.replace((typeof(search)=="string")?new RegExp(search.replace(/[.^$*+?()[{|\\]/g,"\\$&"),"g"):search,(typeof(replace)=="string")?replace.replace(/\$/g,"$$$$"):replace);
    }
}

(Please inform me if it need breakdown and comments)

MDN Content page report details

• Folder: en-us/web/javascript/reference/global_objects/string/replaceall
• MDN URL: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll
• GitHub URL: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/string/replaceall/index.md
• Last commit: 603c0bd
• Document last modified: 2021-08-03T03:20:47.000Z

[digi-booster]

I think Chris Ferdinand's String.replaceAll() polyfill is much easier to understand.

/**
 * String.prototype.replaceAll() polyfill
 * @author Chris Ferdinandi
 * @license MIT
 */
if (!String.prototype.replaceAll) {
	String.prototype.replaceAll = function(str, newStr){

		// If a regex pattern
		if (Object.prototype.toString.call(str).toLowerCase() === '[object regexp]') {
			return this.replace(str, newStr);
		}

		// If a string
		return this.replace(new RegExp(str, 'g'), newStr);

	};
}

[PAPIONbit]

@digi-booster thanks for the responding.
Yes but have serious issues:

1. Not safe: The search (str) can have reserved characters [.^$*+?()[{|\\] so it is not right to directly be used as a RegExp pattern.
2. Hard way to check type of input: The Object.prototype.toString.call(str).toLowerCase() can be replaced by str instanceof RegExp
3. Not reduced/optimized: If the if statement be one-line by using short-hand the process will be much faster.

Also thank you again because the idea made me realize that my way have some issues too:

1. typeof will detect RegExp and new String() as object. So it better to i use search instanceof RegExp
2. It is wrong that i try to replace $ with $$ for being safe on input for replace. If the user use $1 as a group match it should work, because sure he have to know what is the meaning if he used a RegExp as search. (My function was made for personal use and after years exactly replaceAll turn to a native function)

Result (Easiest for understanding):

if (!String.prototype.replaceAll) { // Check if a native function exist
    String.prototype.replaceAll = function(search, replace) { // Define the function by closest input names (For good info in consoles)
        return this.replace(
          (search instanceof RegExp) ? search // Check if the input is already a RegExp then pass it
          	: new RegExp( String(search).replace(/[.^$*+?()[{|\\]/g, "\\$&") , "g"), // Else, replace all reserved characters with '\' then make a global 'g' RegExp
          replace);
    }
}

Also we can check if the input RegExp is global or not then throw an error. But i think even the error is not necessary.

[teoli2003]

For polyfills, I'm ok to link to one as this feature is from 2020, but we shouldn't host it on MDN as we can't really maintain them. So we want is an externally-hosted polyfill that we will link to in the Browser Compatibility section below the table.

[PAPIONbit]

@teoli2003 Thank you.
If you mean this:
https://github.com/zloirock/core-js/blob/master/packages/core-js/modules/es.string.replace-all.js
I think it do not have educational aspect, and a general library is not what we are looking for in a reference.
It is not a big deal, but i think it need a revision. Regards

[zloirock]

Such incomplete and not completely correct in some cases polyfills cause too many problems since someone tries to use them in production. After that, we have issues like this zloirock/core-js#341

[PAPIONbit]

@zloirock Dear Denis, i am generally agree with you. Also i am not agree to let libraries/plugins touch or block native functions, if their are already exist!
But the issue is something else here. Clearly i asked why introducing a big library? when with a simple example that work right, we can have educational propose.

[zloirock]

@PAPIONbit polyfills should fix incorrect implementations that already exist - it's how they work. Your example works not as should work by the spec in too many cases, more other - it adds an enumerable property to String prototype that could cause additional problems.

Yes, the educational purpose is good - but as I wrote above - too many people try to use polyfills from MDN in production.

Yes, introducing a big library is not required in many cases - but if it's not 100% proper by the spec as it could - it should not pollute the prototype for avoiding usage of this code in cases that it could break. It could be a function-helper, but not a property of the String prototype.

[PAPIONbit]

@zloirock

@PAPIONbit polyfills should fix incorrect implementations that already exist - it's how they work.

Yes, If they really can handle it 100% (as you said). In your case could core-js do it? No, just because of a new option in setting properties.
pollyfills have to fill something that missed and handle it as much as possible. And the much, should worth it.
Why?
Because we already working in scripting level to don't bother our-self for managing hardware and solved issues again and again, to focus on just our plans. So if a wrong decide (Even by professionals - Even in ECMAScript) make the scripting harder, i am not agree with that and will go to make it easier (In our libraries) as much as it worth and possible.

My simple suggestion as i said is not a big deal but

but if it's not 100% proper by the spec as it could - it should not pollute the prototype

1. I would be thankful if you explain what is use of replaceAll in case of using RegEx with global flag by replace ?!
2. Except in the case that the polyfill do not handle the error, Where the function make mistake (the pollute)? (It will be great if a better but independent polyfill example for the function be suggested, by a professional like you)
3. Can core-js handle it automatically (without your hard work on the update) if in new ESCMAScript they going to remove or handle the error (Error of not global flag exist)

I really appreciate your efforts, but the option (Changing native properties) wisely exist till today to if we can, manage native functions for solving problems as fast and easiest that possible, we do it.
Options exist, and developers can check and decide.
Best Regards

[zloirock]

In your case could core-js do it? No, just because of a new option in setting properties.

What do you mean?

So if a wrong decide (Even by professionals - Even in ECMAScript) make the scripting harder, i am not agree with that and will go to make it easier (In our libraries) as much as it worth and possible.

Not a problem, but only even in the scope of your code that's not exposed outside - overwise, it should maximally follow the standard or it will cause significant problems - I see it too often.

I would be thankful if you explain what is use of replaceAll in case of using RegEx with global flag by replace ?!

You could ask it, for example, the authors of this proposal, I'm just an implementor. I can assume, for example, that this shows intent more clearly. The main use case, sure, is a string argument.

Except in the case that the polyfill do not handle the error, Where the function make mistake (the pollute)?

What do you mean?

It will be great if a better but independent polyfill example for the function be suggested, by a professional like you

You added a link above - it's not a big problem to implement the required abstract ECMAScript operations out of scope core-js. However, a proper independent polyfill anyway will take a couple of hundreds of lines, so I'm not sure that makes sense to add it to MDN.

Can core-js handle it automatically (without your hard work on the update) if in new ESCMAScript they going to remove or handle the error (Error of not global flag exist)

Yes, sure, since now it's missed in the feature detection. However, I don't think that they will do it - it was added only in the final stages.

I really appreciate your efforts, but the option (Changing native properties) wisely exist till today to if we can, manage native functions for solving problems as fast and easiest that possible, we do it.
Options exist, and developers can check and decide.

"Wisely" here is the main word - and it's the main problem that too many developers do not understand what's allowed and what can break the Web.

[PAPIONbit]

@zloirock You got me wrong, Also not friendly. These kind of words will not make the issue more clear, so the subject is finished for me after this:

"Wisely" here is the main word - and it's the main problem that too many developers do not understand what's allowed and what can break the Web.

Bravo that you are wise.

Just to respect your response:

• The first What do you mean? SCRIPT5045: Assignment to read-only properties is not allowed in strict mode IE11 zloirock/core-js#341 (comment)
  If any other code, change the configurable, can core-js handle it 100%? Or 90%? What if in feature the find have more arguments? and etc... Clearly i say impossible you expect 100% from a script level library. So because of the level, let choose the simple and fast that will work.

So if a wrong decide (Even by professionals - Even in ECMAScript) make the scripting harder, i am not agree with that and will go to make it easier (In our libraries) as much as it worth and possible.

Not a problem, but only even in the scope of your code that's not exposed outside - overwise, it should maximally follow the standard or it will cause significant problems - I see it too often.

You could ask it, for example, the authors of this proposal, I'm just an implementor. I can assume, for example, that this shows intent more clearly. The main use case, sure, is a string argument.

So you mean lets we do not use our minds for public exposes? It means you are going to indirectly tell to let not use a library's like core-js or at least let not expose them in MDN? (Because everyday we find some bugs and try to fix them so we still did not prove the maximum standard, so maybe an other developer can handle it better, also even we made some new standards in core-js etc...)
That's wrong. But your word is generally right.
So when we can not be 100% accurate, logically we suggest the closest to standard & best way that is possible, But the possibility should follow this unwritten rule that all of us know in scripting level: Keep it simple, easy and understandable.

Don't give me wrong again, i am not going to say this:
#9871 (comment)
is better than this:
https://github.com/zloirock/core-js/blob/master/packages/core-js/modules/es.string.replace-all.js
Never, just the first will handle it just OK. And also never break any right standard and never blocked the way to you redefine it again. Also MDN is not there to introduce or choose products for people, it is there to give them references with educational propose. (Even when sometimes the content is not mentioned in W3C but it is trustable.)
So
First you have to make it clear, where is the many problems of my suggested function. (except about throwing the error that can be handled easy but for first step i did not, as i explained) (I eagerly will listen to it - If necessary, it can be upgraded without being complicated)
Second you answered yourself, if:

However, a proper independent polyfill anyway will take a couple of hundreds of lines,

So? :)

Yes, sure

No, Sure. Because logically you can not know future. (Years ago, I was telling same things to my friends about why not trust JQuery too.)

• The second What do you mean? (I talked about that up there but again):

Also we can check if the input RegExp is global or not then throw an error. But i think even the error is not necessary.

.

[zloirock]

You got me wrong, Also not friendly.

Rather your answer sounds not friendly. I'm just trying to explain the dangers of naive global polyfills.

If any other code, change the configurable, can core-js handle it 100%? Or 90%? What if in feature the find have more arguments? and etc... Clearly i say impossible you expect 100% from a script level library. So because of the level, let choose the simple and fast that will work.

Polyfills should not handle such cases since it's a case of an environment broken in the userland. It's strange to expect that if you break your environment, like delete window.Array, core-js will work. It's impossible to foresee any bullshit that can be done in the userland - polyfills should not do it.

Clearly i say impossible you expect 100% from a script level library. So because of the level, let choose the simple and fast that will work.

Not in the case of something that pollutes global. Just remember why Array#includes is called not Array#contains of Array#flat is not Array#flatten.

So you mean lets we do not use our minds for public exposes? It means you are going to indirectly tell to let not use a library's like core-js or at least let not expose them in MDN?

I mean only that it's a bad practice and dangerous to publish not maximally proper global polyfills. It's fine to publish proper polyfills as packages, for example, es-shims also propose proper polyfills. It's OK for me to publish maximally proper compact polyfills on MDN. But now the most think that MDN should not store polyfills code.

So when we can not be 100% accurate...

We can be 100% accurate in most cases when no one breaks global in userland, and naive polyfills are a case that could break it.

First you have to make it clear, where is the many problems of my suggested function.

I even don't know what to say. For example, from adding an enumerable property to String prototype to instanceof instead of IsRegExp check or String instead of ToString conversion.

However, a proper independent polyfill anyway will take a couple of hundreds of lines,

So? :)

See the link above.

if in new ESCMAScript they going to remove or handle the error (Error of not global flag exist)

No, Sure. Because logically you can not know future.

Yes, sure, since it's the answer to your concrete example from the question and I perfectly know how it works in this case. More other, it will break future standard features only if those standards will break the current userland code that's impossible - those features just will not be added in this case.

[PAPIONbit]

@zloirock Dear Denis! If you be honest and calm down, you will get what am i saying. I said the subject between you and me closed for me. Because of expecting a rude behavior that is not depend to your high & professional personality:

... any bullshit that ...

!

(Actually i am not interested anymore to even talk, but you answered. and i try to focus on parts that is useful.)

Rather your answer sounds not friendly

(I think you have to check again without prejudice)

---

Polyfills should not handle such cases since...

If you do not try to caricaturize it (delete window!!!), my word was clear, and that was just an example from what you mention it.
I was/am completely agree with prove of maximum of standard in any case. (Just think actually the subject is not about this)
It depend to your viewpoint about position/level of scripting. (We need reasons, not unclear personal opinions.)
The environment is editable as it wisely should be.
Can we call Modified environment as Broken, just because core-js can not handle it? We can not. We just can say: It is what it is.
Because developers have this right to know about it and do it. For the reasons that i said in last posts.
You don't like to face it (A bad modified environment)? You are right, Me too!
But is there any really standard to we be 100% sure about it? No, Not here, even not in world. Just look how many great libraries try to handle a same subject with different ways. And we can not say the creators of them are not professionals too.
It is why i said a reference should/can not mention an unofficial/private product like core-js or anything else. Still it can be reasonable if in part of the product independently we can focus on the subject and it explained well to we do not miss the educational propose. (But in our case, is it?)
So generally we have to let anyone (Developer/User) deiced about it by himself, even we have to give them options that maybe we don't like, but possible (or it is their rights), because it is exactly what WEB going to hold it.
Now if someone going to use core-js, it is his fault to get an error if he did not read your documents well and touched his default environment (by a non-standard library or directly). Just that and nothing more. (=We can not search the problem somewhere else)
And if someone going to not use any library and believe in this level of programming (scripting) we do not need a big library for simple tasks, is there any option to he can refer to? (I saw it here long time ago, so with some lite improvement i share it.)
I was going to just make that option till the level that it look simple and worth it. (If it need improvements, i can mention it as known bugs, OR i can upgrade it by sharing it here and use other professionals opinion)

Yes, sure, since it's the answer...

I was not talking about what we do about future, I mean we have to accept, our hands (As developers in script level) are closed if for example ECMAScript deciders change their mind. That's a fact.

I mean only that it's a bad practice...

See the link above.

You can never have double standard, If It can not be officially referred / It can have serious bugs and not trusted / It not worth it / ...
Exactly if you agree with fscholz, we have to remove your link to your library.
I am 80% agree with this but i believe the problem can be solved by mention it as a non-standard way or something like that. Because in many cases, logically it is impossible you reach exact native functions. (+ I like the idea of MDN polyfill)

I even don't know what to say...

You know what to say :), You said well (IT is friendly, Thank you). So:

• Using instanceof for IsRegExp was naive. You are right. (https://github.com/sindresorhus/is#why-not-just-use-instanceof-instead-of-this-package)
• String() instead of .toString is not naive but i am not sure if it go wrong in service workers. I wanted to handle null too.

More?

I think these:

• from this line till end
• [RegExp] Incomplete sticky flag support implementation / [email protected]+ breaks [email protected] patterns on IE11 zloirock/core-js#810

Will give me more evidence to it worth looking at the subject again.

In case of we accept we do not have to accept the standard of throwing error because the regex is not global is not necessary.
Now it is an OK ployfill:

if (!String.prototype.replaceAll) {
    String.prototype.replaceAll = function (search, replace) {
        return this.replace( Object.prototype.toString.call(x) === "[object String]" ? new RegExp( search.replace(/[.^$*+?()[{|\\]/g, "\\$&"), "g") : search, replace);
    }
}

Is it?

---

core-js worth to be seen and used. It is my opinion. (It don't need my admire ;))
I found you a great programmer, (You don't need my admire ;))
I think we all have possible mistakes ( me more :) ), and we need patience for it, instead of any kind of confrontation.
Best Regards

[zloirock]

!

Distributing a non-configurable broken polyfill is bullshit, obviously. I don't understand why this offends you.

---

Too many useless words. We can't call such a broken environment "modified", it's just broken. It is generally recognized as bad practice. Modification of global prototypes is allowed only to maximally proper polyfills. This is a fact and it is not negotiable.

zloirock/core-js#810

I can't understand, what are you trying to show? core-js contains 395 polyfills modules and it's the only known bug - and I don't fix it by myself principally - I didn't write this polyfill module and it's not my bug. If you wanna - you could fix it - it's marked as help wanted.

So

IsRegExp !== not a string. ToString !== .toString.

Now it is an OK polyfill

Nope, the previous option was better.

Just a simple example that will break too much userland code, I already 2 times wrote about enumerability:

for (var key in '') console.log(key); // => 'replaceAll'

In case you were adding it as a non-enumerable property - I wouldn't even start this discussion.

[zloirock]

I do not intend to continue this useless conversation, good luck.

[PAPIONbit]

It seems more appropriate that MDN avoid to mention an external link to a Private/Unofficial library/plugin as polyfill. Because:

• Possible mistake/bugs or future modifications of them. (while our inaccessibility)
• Inability to examine all libraries/plugins for a correct and fair introduction. (Also in case of introducing some, taking chance of being seen from others.)
• Missing Educational aspect in general

Also i think independent polyfills still better be exist in main page of MDN references, but:

• As a subset of Browser compatibility
• By mentioning their's:
  • Possible difference with the native function
  • Limitations
  • How & Where use/not_use

My suggested options for replaceAll

With global-flag error: (More principled)

if (!String.prototype.replaceAll) { // Check if a native function exist
    String.prototype.replaceAll = function(search, replace) { // Define the function by closest input names (For good info in consoles)
        return this.replace(
          Object.prototype.toString.call(search) === '[object RegExp]' // IsRegExp?
          	? search.global // Is the RegEx global?
            	    ? search // So pass it
            	    : function(){throw new TypeError('replaceAll called with a non-global RegExp argument')}() // If not throw an error
          	: new RegExp( String(search).replace(/[.^$*+?()[{|\\]/g, "\\$&"), "g"), // Else, replace all reserved characters with '\' then make a global 'g' RegExp from the string
          replace); // passing second argument
    }
}

With handling global-flag missing by itself: (My first preference)

if (!String.prototype.replaceAll) { // Check if a native function exist
    String.prototype.replaceAll = function(search, replace) { // Define the function by closest input names (For good info in consoles)
        return this.replace(
          Object.prototype.toString.call(search) === '[object RegExp]' // IsRegExp?
          	? search.global // Is the RegEx global?
            	    ? search // So pass it
            	    : new RegExp( search.source, /\/([a-z]*)$/.exec(search.valueOf())[1] +'g') // If not, make a global clone from the RegEx
          	: new RegExp( String(search).replace(/[.^$*+?()[{|\\]/g, "\\$&"), "g"), // Else, replace all reserved characters with '\' then make a global 'g' RegExp
          replace); // passing second argument
    }
}

Without global-flag error: (My second preference)

if (!String.prototype.replaceAll) { // Check if a native function exist
    String.prototype.replaceAll = function(search, replace) { // Define the function by closest input names (For good info in consoles)
        return this.replace( // Using native String.prototype.replace()
          Object.prototype.toString.call(search) === '[object RegExp]' // IsRegExp?
                ? search // So pass it
          	: new RegExp( String(search).replace(/[.^$*+?()[{|\\]/g, "\\$&"), "g"), // Else, replace all reserved characters with '\' then make a global 'g' RegExp from the string
          replace); // passing second argument
    }
}

Browser support:
IE 8+ (Tested on IE11)
(+) All other browsers (>2012)

As i tested, the result is same as the native replaceAll in case of first argument input be:
null, Object, Function, Date, ... , RegExp, Number, String

Please let me know in case of any wrong/error.

[sideshowbarker]

It seems more appropriate that MDN avoid to mention an external link to a Private/Unofficial library/plugin as polyfill.
…
Also i think independent polyfills still better be exist in main page of MDN references

As already explained in #9871 (comment), we’re not going to host polyfills any longer in any part of MDN.

I’m going ahead and closing this, since the conversation in the comments doesn’t seem to be leading toward a resolution.