Add minimal specification #13
Conversation
This is the smallest possible delta compared to String#replace that I could think of. This approach takes the _searchValue_ and turns it into a global regular expression if it isn't already, carefully escaping special characters when needed. It throws when RegExp.prototype[Symbol.replace] is not callable (e.g. when it has been deleted). This way, we avoid the introduction of RegExp.prototype[Symbol.replaceAll] which would basically do the same thing as RegExp.prototype[Symbol.replace] for global regular expressions anyway.
It's redundant, since `Call` already throws when needed.
|
Some more context: https://tc39.github.io/ecma262/#sec-well-known-symbols currently says:
That description matches perfectly with not just |
|
Preserving offline discussion: I think we should keep the string-pattern-matching-only path (= what String.p.replace currently does when searchValue doesn't have a @@replace symbol). Passing a string searchValue should not have side effects, and constructing a RegExp + calling exec on it is observable through global properties set on RegExp (e.g. RegExp.input, RegExp.lastMatch, ...). |
|
I was hoping to avoid it, but forgot about the global |
| 1. If _isRegExp_ is *true*, then | ||
| 1. Let _flags_ be _searchValue_.[[OriginalFlags]]. | ||
| 1. If _flags_ does not contain `"g"`, then | ||
| 1. Set _flags_ to a copy of _flags_. |
There was a problem hiding this comment.
Why is this needed? This is a value, not a reference to the internal slot on searchValue.
Alternatively, there’s an existing form of “to the concatenation of a and b” that’s probably better here.
There was a problem hiding this comment.
This is a value, not a reference to the internal slot on searchValue.
That’s not what https://tc39.github.io/ecma262/#sec-algorithm-conventions says:
Algorithm steps may declare named aliases for any value using the form “Let x be someValue”. These aliases are reference-like in that both x and someValue refer to the same underlying data and modifications to either are visible to both. Algorithm steps that want to avoid this reference-like behaviour should explicitly make a copy of the right-hand side: “Let x be a copy of someValue” creates a shallow copy of someValue.
There was a problem hiding this comment.
Interesting, that seems terrifying :-) either way i think the concatenation language would be simpler (it’s what matchAll uses)
| 1. Let _newString_ be the string-concatenation of the first _pos_ code units of _string_, _replStr_, and the trailing substring of _string_ starting at index _tailPos_. If _pos_ is 0, the first element of the concatenation will be the empty String. | ||
| 1. Return _newString_. | ||
| 1. Let _searchString_ be ? ToString(_searchValue_). | ||
| 1. Set _escapedPattern_ to EscapeRegExpPattern(_src_, `"g"`). |
There was a problem hiding this comment.
| 1. Set _escapedPattern_ to EscapeRegExpPattern(_src_, `"g"`). | |
| 1. Set _escapedPattern_ to EscapeRegExpPattern(_searchString_, `"g"`). |
| 1. Set _searchValue_ to ? RegExpCreate(_escapedPattern_, `"g"`). | ||
| 1. Assert: ? IsRegExp(_searchValue_) is *true*. | ||
| 1. Let _replacer_ be ? GetMethod(_searchValue_, @@replace). | ||
| 1. Return ? Call(_replacer_, _searchValue_, « _O_, _replaceValue_ »). |
There was a problem hiding this comment.
this could also just call Invoke directly?
Co-Authored-By: mathiasbynens <mathias@qiwi.be>
| 1. Return _newString_. | ||
| 1. Let _searchString_ be ? ToString(_searchValue_). | ||
| 1. Set _escapedPattern_ to EscapeRegExpPattern(_src_, `"g"`). | ||
| 1. Set _searchValue_ to ? RegExpCreate(_escapedPattern_, `"g"`). |
There was a problem hiding this comment.
So... We add g flag and don't create a copy of regexp with required flags, just use original regexp?
|
Closing this PR since we'll have to go with a more involved solution anyhow. |
diff sans whitespace
This is the smallest possible delta compared to
String#replacethat I could think of. This approach takes the searchValue and turns it into a global regular expression if it isn't already, carefully escaping special characters when needed. It throws whenRegExp.prototype[Symbol.replace]is not callable (e.g. when it has been deleted).This way, we avoid the introduction of
RegExp.prototype[Symbol.replaceAll]which would basically do the same thing asRegExp.prototype[Symbol.replace]for global regular expressions anyway.Thoughts? @psmarshall @schuay @ljharb