From a6a4cb721940ee64ea0392cf42f323287ccc6205 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Wed, 7 Sep 2022 17:49:58 +0800 Subject: [PATCH 1/5] Fix `.replace()` when searching string --- README.md | 4 ++-- src/MagicString.js | 25 +++++++++++++++++++++++-- test/MagicString.js | 23 ++++++++++++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index d57629b..d627a07 100644 --- a/README.md +++ b/README.md @@ -165,9 +165,9 @@ Same as `s.appendLeft(...)`, except that the inserted content will go *before* a Same as `s.appendRight(...)`, except that the inserted content will go *before* any previous appends or prepends at `index` -### s.replace( regexp, substitution ) +### s.replace( regexpOrString, substitution ) -String replacement with RegExp or string, a replacer function is also supported. Returns `this`. +String replacement with RegExp or string. When using a RegExp, replacer function is also supported. Returns `this`. ```ts import MagicString from 'magic-string' diff --git a/src/MagicString.js b/src/MagicString.js index e45ad2e..c7332c4 100644 --- a/src/MagicString.js +++ b/src/MagicString.js @@ -736,7 +736,7 @@ export default class MagicString { return this.original !== this.toString(); } - replace(searchValue, replacement) { + _replaceRegexp(searchValue, replacement) { function getReplacement(match, str) { if (typeof replacement === 'string') { return replacement.replace(/\$(\$|&|\d+)/g, (_, i) => { @@ -759,7 +759,7 @@ export default class MagicString { } return matches; } - if (typeof searchValue !== 'string' && searchValue.global) { + if (searchValue.global) { const matches = matchAll(searchValue, this.original); matches.forEach((match) => { if (match.index != null) @@ -780,4 +780,25 @@ export default class MagicString { } return this; } + + _replaceString(string, replacement) { + const { original } = this; + const stringLength = string.length; + for ( + let index = original.indexOf(string); + index !== -1; + index = original.indexOf(string, index + stringLength) + ) { + this.overwrite(index, index + stringLength, replacement); + } + + return this; + } + + replace(searchValue, replacement) { + return this[typeof searchValue === 'string' ? '_replaceString' : '_replaceRegexp']( + searchValue, + replacement + ); + } } diff --git a/test/MagicString.js b/test/MagicString.js index 4bc1d09..c86b7ab 100644 --- a/test/MagicString.js +++ b/test/MagicString.js @@ -1305,7 +1305,28 @@ describe('MagicString', () => { s.replace('2', '3'); - assert.strictEqual(s.toString(), '1 3 1 2'); + assert.strictEqual(s.toString(), '1 3 1 3'); + }); + + it('Should not treat string as regexp', () => { + assert.strictEqual( + new MagicString('1234').replace('.', '*').toString(), + '1234' + ); + }); + + it('Should use substitution directly', () => { + assert.strictEqual( + new MagicString('11').replace('1', '$0$1').toString(), + '$0$1$0$1' + ); + }); + + it('Should not search back', () => { + assert.strictEqual( + new MagicString('121212').replace('12', '21').toString(), + '212121' + ); }); it('works with global regex replace', () => { From c6ad54a8f7d5e210ea265fe10f24cd2b62a63cf9 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Thu, 22 Sep 2022 15:20:31 +0800 Subject: [PATCH 2/5] Apply review suggestion --- src/MagicString.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/MagicString.js b/src/MagicString.js index c7332c4..e4c91cc 100644 --- a/src/MagicString.js +++ b/src/MagicString.js @@ -796,9 +796,10 @@ export default class MagicString { } replace(searchValue, replacement) { - return this[typeof searchValue === 'string' ? '_replaceString' : '_replaceRegexp']( - searchValue, - replacement - ); + if (typeof searchValue === 'string') { + return this._replaceString(searchValue, replacement); + } + + return this_replaceRegexp(searchValue, replacement); } } From 5c1b35217892484a375f8740ce73f62e4f7fc32a Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Thu, 22 Sep 2022 15:41:27 +0800 Subject: [PATCH 3/5] Add `.replaceAll` --- src/MagicString.js | 29 ++++++++++++++++++--- test/MagicString.js | 63 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/MagicString.js b/src/MagicString.js index e4c91cc..155963f 100644 --- a/src/MagicString.js +++ b/src/MagicString.js @@ -782,6 +782,25 @@ export default class MagicString { } _replaceString(string, replacement) { + const { original } = this; + const index = original.indexOf(string); + + if (index !== -1) { + this.overwrite(index, index + string.length, replacement); + } + + return this; + } + + replace(searchValue, replacement) { + if (typeof searchValue === 'string') { + return this._replaceString(searchValue, replacement); + } + + return this._replaceRegexp(searchValue, replacement); + } + + _replaceAllString(string, replacement) { const { original } = this; const stringLength = string.length; for ( @@ -795,11 +814,15 @@ export default class MagicString { return this; } - replace(searchValue, replacement) { + replaceAll(searchValue, replacement) { if (typeof searchValue === 'string') { - return this._replaceString(searchValue, replacement); + return this._replaceAllString(searchValue, replacement); + } + + if (!searchValue.global) { + throw new TypeError('MagicString.prototype.replaceAll called with a non-global RegExp argument'); } - return this_replaceRegexp(searchValue, replacement); + return this._replaceRegexp(searchValue, replacement); } } diff --git a/test/MagicString.js b/test/MagicString.js index c86b7ab..c2b9c5b 100644 --- a/test/MagicString.js +++ b/test/MagicString.js @@ -1305,7 +1305,7 @@ describe('MagicString', () => { s.replace('2', '3'); - assert.strictEqual(s.toString(), '1 3 1 3'); + assert.strictEqual(s.toString(), '1 3 1 2'); }); it('Should not treat string as regexp', () => { @@ -1318,13 +1318,13 @@ describe('MagicString', () => { it('Should use substitution directly', () => { assert.strictEqual( new MagicString('11').replace('1', '$0$1').toString(), - '$0$1$0$1' + '$0$11' ); }); it('Should not search back', () => { assert.strictEqual( - new MagicString('121212').replace('12', '21').toString(), + new MagicString('122121').replace('12', '21').toString(), '212121' ); }); @@ -1368,4 +1368,61 @@ describe('MagicString', () => { ); }); }); + + describe('replaceAll', () => { + it('works with string replace', () => { + assert.strictEqual( + new MagicString('1212').replaceAll('2', '3').toString(), + '1313', + ); + }); + + it('Should not treat string as regexp', () => { + assert.strictEqual( + new MagicString('1234').replaceAll('.', '*').toString(), + '1234' + ); + }); + + it('Should use substitution directly', () => { + assert.strictEqual( + new MagicString('11').replaceAll('1', '$0$1').toString(), + '$0$1$0$1' + ); + }); + + it('Should not search back', () => { + assert.strictEqual( + new MagicString('121212').replaceAll('12', '21').toString(), + '212121' + ); + }); + + it('global regex result the same as .replace', () => { + assert.strictEqual( + new MagicString('1 2 3 4 a b c').replaceAll(/(\d)/g, 'xx$1$10').toString(), + new MagicString('1 2 3 4 a b c').replace(/(\d)/g, 'xx$1$10').toString(), + ); + + assert.strictEqual( + new MagicString('1 2 3 4 a b c').replaceAll(/(\d)/g, '$$').toString(), + new MagicString('1 2 3 4 a b c').replace(/(\d)/g, '$$').toString(), + ); + + assert.strictEqual( + new MagicString('hey this is magic').replaceAll(/(\w)(\w+)/g, (_, $1, $2) => `${$1.toUpperCase()}${$2}`).toString(), + new MagicString('hey this is magic').replace(/(\w)(\w+)/g, (_, $1, $2) => `${$1.toUpperCase()}${$2}`).toString(), + ); + }); + + it('rejects with non-global regexp', () => { + assert.throws( + () => new MagicString('123').replaceAll(/./, ''), + { + name: 'TypeError', + message: 'MagicString.prototype.replaceAll called with a non-global RegExp argument', + }, + ); + }); + }); }); From c270716e1bf5364714c38daa8340585a6259b42d Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Thu, 22 Sep 2022 15:47:07 +0800 Subject: [PATCH 4/5] Update docs --- README.md | 5 +++++ src/MagicString.js | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d627a07..e052919 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,11 @@ Same as `s.appendRight(...)`, except that the inserted content will go *before* String replacement with RegExp or string. When using a RegExp, replacer function is also supported. Returns `this`. +### s.replaceAll( regexpOrString, substitution ) + +Same as `s.replace`, but replace all matched strings instead of just one. +If `substitution` is a regex, then it must have the global (`g`) flag set, or a `TypeError` is thrown. Matches the behavior of the bultin [`String.property.replaceAll`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll). + ```ts import MagicString from 'magic-string' diff --git a/src/MagicString.js b/src/MagicString.js index 155963f..2a6fe77 100644 --- a/src/MagicString.js +++ b/src/MagicString.js @@ -820,7 +820,9 @@ export default class MagicString { } if (!searchValue.global) { - throw new TypeError('MagicString.prototype.replaceAll called with a non-global RegExp argument'); + throw new TypeError( + 'MagicString.prototype.replaceAll called with a non-global RegExp argument' + ); } return this._replaceRegexp(searchValue, replacement); From fd3c8cc84488181d4f47fa2a1a8693478c84b612 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Thu, 22 Sep 2022 15:55:53 +0800 Subject: [PATCH 5/5] Fix docs --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e052919..1b63f34 100644 --- a/README.md +++ b/README.md @@ -169,11 +169,6 @@ Same as `s.appendRight(...)`, except that the inserted content will go *before* String replacement with RegExp or string. When using a RegExp, replacer function is also supported. Returns `this`. -### s.replaceAll( regexpOrString, substitution ) - -Same as `s.replace`, but replace all matched strings instead of just one. -If `substitution` is a regex, then it must have the global (`g`) flag set, or a `TypeError` is thrown. Matches the behavior of the bultin [`String.property.replaceAll`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll). - ```ts import MagicString from 'magic-string' @@ -188,6 +183,11 @@ The differences from [`String.replace`]((https://developer.mozilla.org/en-US/doc - It will always match against the **original string** - It mutates the magic string state (use `.clone()` to be immutable) +### s.replaceAll( regexpOrString, substitution ) + +Same as `s.replace`, but replace all matched strings instead of just one. +If `substitution` is a regex, then it must have the global (`g`) flag set, or a `TypeError` is thrown. Matches the behavior of the bultin [`String.property.replaceAll`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll). + ### s.remove( start, end ) Removes the characters from `start` to `end` (of the original string, **not** the generated string). Removing the same content twice, or making removals that partially overlap, will cause an error. Returns `this`.