5 ms·
I don’t understand the rationale behind throwing an exception if you provide a regular expression which lacks the g modifier. This seems really counter intuitiv
by bendiksolheim 7y ago
I don’t understand the rationale behind throwing an exception if you provide a regular expression which lacks the g modifier. This seems really counter intuitive. I would expect replaceAll to replace all occurrences no matter if the regular expression is global or not.
- TAForObvReasons 7y ago> The assumption is that we made a mistake and should switch to .replace() if we really only want to replace the first occurrence. Unlike the string case, the regexp case has a way of communicating that you want to match every instance versus only the first
- bendiksolheim 7y agoThat is an interesting way of seeing it. I’m not saying I disagree with the rationale, but I find it weird that a function which promises to do something needs a specially modified version of its input _not_ throw an exception. That does not sound like a good API design at all.
- TheRealPomax 7y agoThis feels like thinking about it the wrong way round: if you want to use regular expressions, use `.replace()`, which has full regex support and always has. There is no need to ever use `replaceAll` with regular expressions. In fact, you'd just be mudding the waters by using a function that implies "all" in combination with a regex that implies "single" (if you don't specify a `g` flag, as per regex convention). However, if you just want to replace plain strings, `.replaceAll()` actually gives you a fundamentally better API: you can now use `str.replace(s,r)` for single replace, and the intuitive companion function `str.replaceAll(s,r)` for all-instances replace. No "learning/knowing regexp" required just to replace some plain text.
- bendiksolheim 7y agoIf there is no need in using replaceAll() with regexes, why would it even support it? I agree that only replacing the first occurrence is the wrong solution. That would be surprising. The only correct solution to this is in my opinion auto-g-ing the regular expression, no matter how unusual that is. There is no way you would write replaceAll() as a mistake when you actually just meant replace().
- TheRealPomax 7y agoMostly to keep the signature for replaceAll in line with replace. You _can_ call it with a regex, but then you'll have to make sure it's a regex that uses the correct syntax for "replace-all" behaviour. If you don't, now you're doing something so weird that an error is the only appropriate response. <<There is no way you would write replaceAll() as a mistake when you actually just meant replace(). >> Have you ever reviewed the daily work of colleagues or contributors who haven't used JS day in day for the last 2 years? That's exactly the kind of mistake that happens _all_ the time. And people new to JS are more than happy to reach for regex well before there's a good reason to use them.
- a13n 7y agoIs there a way to find all occurrences/matches of a regex that doesn't have the 'g' modifier? Just tried `string.match` and `regex.exec` and could only get the first match.
- zamadatix 7y agoI don't think so without modifying the string or regex each iteration. You'd have to do something like let flagified = new RegExp(original.source, original.flags + "g") or modify the regex standards as part of the proposal.
- chipperyman573 7y agoI tried to do this a while ago for a side project. There might be a really obscure way (and of course you can always write your own function), but I wasn't able to find anything built in.
- snek 7y agoIt has to do with certain behaviours of RegExp objects, such as the `lastIndex` property, as well as consistency with `matchAll`. There was a lot of discussion[1] about what should happen with a regex that doesn't have /g, and in the end the most consistent choice was to just disallow them entirely. [1]: https://github.com/tc39/proposal-string-replaceall/issues/16 https://github.com/tc39/proposal-string-replaceall/issues/16
- dmitriid 7y agoThe irony in that discussion regarding behavior of matchAll: "Once something has shipped in browser's we can't change it". Chrome: deprecating and removing Custom Elements V0 in 2020.
- explodingcamera 7y ago*V0, this has always been explicitly unstable
- vimslayer 7y agoAlso, Custom Elements is in the area of W3C, not TC39, I believe. There could be different rules and people involved. Although I'm sure they'd like to avoid breaking the web too.
- dmitriid 7y agoIndeed, and that's the point I failed to make. TC39 is rightfully concerned about not breaking the web. They are very mindful even if features that have reached stage 3 (because by this time everyone is already using them). Browser implementors though... They take a much more callous approach.
- dmitriid 7y agoAnd used by the biggest websites and projects on the Web (Google's own YouTube and AMP until very recently). How many more websites are using it because it was out, and implemented?
- happytoexplain 7y agoI too was surprised by this, but for the opposite reason: I would expect replaceAll to replace all matches for the regex, meaning if the regex only matches the first occurrence of a pattern, then only the first occurrence is replaced. The fact that we both made perfectly opposite assumptions about what is "obviously right" is strong evidence for their decision.
- chipperyman573 7y agoIsn't that how .replace works? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
- happytoexplain 7y agoPerhaps - I rarely write JS. I'm speaking purely based on this function alone.
- bendiksolheim 7y agoI disagree that this makes their decision the right one. If we can come up with so many "valid" versions of this, all with their own flaws, then this is the wrong API to build. This function should not exist in its current form at all. APIs should be intuitive, preferably without reading the documentation (although I agree that this is a utopian way of seeing it).
- tmpfile 7y agoI agree. I would think replaceAll would behave like the following: /* Usage: > 'abcdefabc'.replaceAll('abc') 'def' > 'abcdefabc'.replaceAll('abc', '+') '+def+' > 'abcdefabc'.replaceAll(/abc/, '+') '+def+' > 'abcdefabc'.replaceAll(/abc/g, '+') '+def+' */ String.prototype.replaceAll = function (re_or_str, sub) { return this.split(re_or_str).join(sub||'') } On a side note, is it just me or does the linked polyfill for String.prototype.replaceAll [1] seem wildly complex??? [1] https://github.com/es-shims/String.prototype.replaceAll/blob/master/implementation.js https://github.com/es-shims/String.prototype.replaceAll/blob...
- jsjohnst 7y agoAgree, seems much more complex than it should need to be, but I’m guessing there’s edge cases (but even then it seems more complex than needed).
- vimslayer 7y agoYour implementation is missing features like capture-based substitions // Remove repeated characters 'foobar'.replace(/(.)\1+/g, '$1') "fobar" and overriding behaviour with Symbol.replace
- deleted 7y ago[deleted]
- tmpfile 7y agoYou're right, my implementation was not a drop-in replacement for replace. How about this? String.prototype.replaceAll = function (n, sub) { let t, f = "g"; if (n instanceof RegExp) { t = n.source; f = n.flags; if (!f.includes("g")) f += "g"; } else { t = RegExp.escape(n); } let r = new RegExp(t, f); return this.replace(r, sub || ""); }
- tus88 7y agoHow do you know when an entire language is ready to be thrown in the trashcan? When you are having a debate about what "all" means.