Skip to content

Commit

Permalink
String#{ matchAll, replaceAll } throws a error on non-global regex …
Browse files Browse the repository at this point in the history
…argument

per tc39/proposal-string-replaceall#24 and
decision from TC39 meetings
  • Loading branch information
zloirock committed Sep 28, 2019
1 parent e4505d2 commit 9ab0f3a
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## Changelog
##### Unreleased
- **`String#{ matchAll, replaceAll }` throws a error on non-global regex argument per [this PR](https://github.com/tc39/proposal-string-replaceall/pull/24) and decision from TC39 meetings. It's a breaking change, but because it's a breaking change in the ES spec, it's added in the minor release**
- Added a workaround for iOS Safari MessageChannel + bfcache bug, [#624](https://github.com/zloirock/core-js/issues/624)
- Added a workaround for Chrome 33 / Android 4.4.4 `Promise` bug, [#640](https://github.com/zloirock/core-js/issues/640)
- Added compat data for Node 12.9, FF 69 and Phantom 1.9
Expand Down
7 changes: 4 additions & 3 deletions packages/core-js-compat/src/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -848,9 +848,10 @@ const data = {
safari: '10.0',
},
'es.string.match-all': {
chrome: '73',
firefox: '67',
safari: '13',
// Early implementations does not throw an error on non-global regex
// chrome: '73',
// firefox: '67',
// safari: '13',
},
'es.string.pad-end': {
edge: '15',
Expand Down
22 changes: 17 additions & 5 deletions packages/core-js/modules/es.string.match-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ var toLength = require('../internals/to-length');
var aFunction = require('../internals/a-function');
var anObject = require('../internals/an-object');
var classof = require('../internals/classof');
var getFlags = require('../internals/regexp-flags');
var isRegExp = require('../internals/is-regexp');
var getRegExpFlags = require('../internals/regexp-flags');
var createNonEnumerableProperty = require('../internals/create-non-enumerable-property');
var fails = require('../internals/fails');
var wellKnownSymbol = require('../internals/well-known-symbol');
var speciesConstructor = require('../internals/species-constructor');
var advanceStringIndex = require('../internals/advance-string-index');
Expand All @@ -21,6 +23,11 @@ var setInternalState = InternalStateModule.set;
var getInternalState = InternalStateModule.getterFor(REGEXP_STRING_ITERATOR);
var RegExpPrototype = RegExp.prototype;
var regExpBuiltinExec = RegExpPrototype.exec;
var nativeMatchAll = ''.matchAll;

var WORKS_WITH_NON_GLOBAL_REGEX = !!nativeMatchAll && !fails(function () {
'a'.matchAll(/./);
});

var regExpExec = function (R, S) {
var exec = R.exec;
Expand Down Expand Up @@ -64,7 +71,7 @@ var $matchAll = function (string) {
C = speciesConstructor(R, RegExp);
flagsValue = R.flags;
if (flagsValue === undefined && R instanceof RegExp && !('flags' in RegExpPrototype)) {
flagsValue = getFlags.call(R);
flagsValue = getRegExpFlags.call(R);
}
flags = flagsValue === undefined ? '' : String(flagsValue);
matcher = new C(C === RegExp ? R.source : R, flags);
Expand All @@ -76,15 +83,20 @@ var $matchAll = function (string) {

// `String.prototype.matchAll` method
// https://github.com/tc39/proposal-string-matchall
$({ target: 'String', proto: true }, {
$({ target: 'String', proto: true, forced: WORKS_WITH_NON_GLOBAL_REGEX }, {
matchAll: function matchAll(regexp) {
var O = requireObjectCoercible(this);
var S, matcher, rx;
var flags, S, matcher, rx;
if (regexp != null) {
if (isRegExp(regexp)) {
flags = String('flags' in RegExpPrototype ? regexp.flags : getRegExpFlags.call(regexp));
if (!~flags.indexOf('g')) throw TypeError('`.matchAll` does not allow non-global regexes');
}
if (WORKS_WITH_NON_GLOBAL_REGEX) return nativeMatchAll.apply(O, arguments);
matcher = regexp[MATCH_ALL];
if (matcher === undefined && IS_PURE && classof(regexp) == 'RegExp') matcher = $matchAll;
if (matcher != null) return aFunction(matcher).call(regexp, O);
}
} else if (WORKS_WITH_NON_GLOBAL_REGEX) return nativeMatchAll.apply(O, arguments);
S = String(O);
rx = new RegExp(regexp, 'g');
return IS_PURE ? $matchAll.call(rx, S) : rx[MATCH_ALL](S);
Expand Down
29 changes: 10 additions & 19 deletions packages/core-js/modules/esnext.string.replace-all.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,31 @@
'use strict';
var $ = require('../internals/export');
var createNonEnumerableProperty = require('../internals/create-non-enumerable-property');
var requireObjectCoercible = require('../internals/require-object-coercible');
var anObject = require('../internals/an-object');
var isRegExp = require('../internals/is-regexp');
var getRegExpFlags = require('../internals/regexp-flags');
var speciesConstructor = require('../internals/species-constructor');
var wellKnownSymbol = require('../internals/well-known-symbol');
var IS_PURE = require('../internals/is-pure');

var REPLACE_ALL = wellKnownSymbol('replaceAll');
var REPLACE = wellKnownSymbol('replace');
var RegExpPrototype = RegExp.prototype;

var $replaceAll = function (string, replaceValue) {
var rx = anObject(this);
var flags = String('flags' in RegExpPrototype ? rx.flags : getRegExpFlags.call(rx));
if (!~flags.indexOf('g')) {
rx = new (speciesConstructor(rx, RegExp))(rx.source, flags + 'g');
}
return String(string).replace(rx, replaceValue);
};

// `String.prototype.replaceAll` method
// https://github.com/tc39/proposal-string-replace-all
$({ target: 'String', proto: true }, {
replaceAll: function replaceAll(searchValue, replaceValue) {
var O = requireObjectCoercible(this);
var replacer, string, searchString, template, result, index;
var IS_REG_EXP, flags, replacer, string, searchString, template, result, index;
if (searchValue != null) {
replacer = searchValue[REPLACE_ALL];
IS_REG_EXP = isRegExp(searchValue);
if (IS_REG_EXP) {
flags = String('flags' in RegExpPrototype ? searchValue.flags : getRegExpFlags.call(searchValue));
if (!~flags.indexOf('g')) throw TypeError('`.replaceAll` does not allow non-global regexes');
}
replacer = searchValue[REPLACE];
if (replacer !== undefined) {
return replacer.call(searchValue, O, replaceValue);
} else if (IS_PURE && isRegExp(searchValue)) {
return $replaceAll.call(searchValue, O, replaceValue);
} else if (IS_PURE && IS_REG_EXP) {
return String(O).replace(searchValue, replaceValue);
}
}
string = String(O);
Expand All @@ -49,5 +42,3 @@ $({ target: 'String', proto: true }, {
return result;
}
});

IS_PURE || REPLACE_ALL in RegExpPrototype || createNonEnumerableProperty(RegExpPrototype, REPLACE_ALL, $replaceAll);
3 changes: 1 addition & 2 deletions packages/core-js/modules/esnext.symbol.replace-all.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// TODO: remove from `core-js@4`
var defineWellKnownSymbol = require('../internals/define-well-known-symbol');

// `Symbol.replaceAll` well-known symbol
// https://tc39.github.io/proposal-string-replaceall/
defineWellKnownSymbol('replaceAll');
8 changes: 4 additions & 4 deletions tests/commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ for (const _PATH of ['../packages/core-js-pure', '../packages/core-js']) {
ok(load('features/string/trim-end')(' a ') === ' a');
ok(load('features/string/trim-left')(' a ') === 'a ');
ok(load('features/string/trim-right')(' a ') === ' a');
ok('next' in load('features/string/match-all')('a', /./));
ok('next' in load('features/string/match-all')('a', /./g));
ok(typeof load('features/string/replace-all') === 'function');
ok('next' in load('features/string/iterator')('qwe'));
ok(load('features/string/virtual/code-point-at').call('a', 0) === 97);
Expand Down Expand Up @@ -250,7 +250,7 @@ for (const _PATH of ['../packages/core-js-pure', '../packages/core-js']) {
ok(load('features/string/virtual/trim-end').call(' a ') === ' a');
ok(load('features/string/virtual/trim-left').call(' a ') === 'a ');
ok(load('features/string/virtual/trim-right').call(' a ') === ' a');
ok('next' in load('features/string/virtual/match-all').call('a', /./));
ok('next' in load('features/string/virtual/match-all').call('a', /./g));
ok(typeof load('features/string/virtual/replace-all') === 'function');
ok(load('features/string/virtual').at.call('a', 0) === 'a');
ok('next' in load('features/string/virtual/iterator').call('qwe'));
Expand Down Expand Up @@ -529,7 +529,7 @@ for (const _PATH of ['../packages/core-js-pure', '../packages/core-js']) {
ok(load('stable/string/ends-with')('qwe', 'we'));
ok(load('stable/string/includes')('qwe', 'w'));
ok(load('stable/string/repeat')('q', 3) === 'qqq');
ok('next' in load('stable/string/match-all')('a', /./));
ok('next' in load('stable/string/match-all')('a', /./g));
ok(load('stable/string/starts-with')('qwe', 'qw'));
ok(typeof load('stable/string/anchor') === 'function');
ok(typeof load('stable/string/big') === 'function');
Expand Down Expand Up @@ -1595,7 +1595,7 @@ load('features/typed-array/to-string');
load('features/typed-array/values');
ok(typeof load('features/typed-array').Uint32Array === 'function');
ok(typeof load('es/string/match') === 'function');
ok('next' in load('es/string/match-all')('a', /./));
ok('next' in load('es/string/match-all')('a', /./g));
ok(typeof load('es/string/replace') === 'function');
ok(typeof load('es/string/search') === 'function');
ok(load('es/string/split')('a s d', ' ').length === 3);
Expand Down
6 changes: 5 additions & 1 deletion tests/compat/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,11 @@ GLOBAL.tests = {
return ''.match(O) == 7 && execCalled;
},
'es.string.match-all': function () {
return String.prototype.matchAll;
try {
'a'.matchAll(/./);
} catch (error) {
return 'a'.matchAll(/./g);
}
},
'es.string.pad-end': function () {
return String.prototype.padEnd && !WEBKIT_STRING_PAD_BUG;
Expand Down
15 changes: 1 addition & 14 deletions tests/pure/es.string.match-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,7 @@ QUnit.test('String#matchAll', assert => {
value: undefined,
done: true,
});
iterator = matchAll('1111a2b3cccc', /(\d)(\D)/);
assert.isIterator(iterator);
assert.isIterable(iterator);
assert.deepEqual(iterator.next(), {
value: assign(['1a', '1', 'a'], {
input: '1111a2b3cccc',
index: 3,
}),
done: false,
});
assert.deepEqual(iterator.next(), {
value: undefined,
done: true,
});
assert.throws(() => matchAll('1111a2b3cccc', /(\d)(\D)/), TypeError);
iterator = matchAll('1111a2b3cccc', '(\\d)(\\D)');
assert.isIterator(iterator);
assert.isIterable(iterator);
Expand Down
4 changes: 2 additions & 2 deletions tests/pure/esnext.string.replace-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ QUnit.test('String#replaceAll', assert => {
return 'c';
}), 'aca');
const searcher = {
[Symbol.replaceAll](O, replaceValue) {
[Symbol.replace](O, replaceValue) {
assert.same(this, searcher, '`this` is `searcher`');
assert.same(String(O), 'aba', '`O` is `aba`');
assert.same(String(replaceValue), 'c', '`replaceValue` is `c`');
Expand All @@ -29,7 +29,7 @@ QUnit.test('String#replaceAll', assert => {
assert.throws(() => replaceAll(null, 'a', 'b'), TypeError);
assert.throws(() => replaceAll(undefined, 'a', 'b'), TypeError);
}
assert.same(replaceAll('b.b.b.b.b', /\./, 'a'), 'babababab');
assert.throws(() => replaceAll('b.b.b.b.b', /\./, 'a'), TypeError);
assert.same(replaceAll('b.b.b.b.b', /\./g, 'a'), 'babababab');
const object = {};
assert.same(replaceAll('[object Object]', object, 'a'), 'a');
Expand Down
15 changes: 1 addition & 14 deletions tests/tests/es.string.match-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,7 @@ QUnit.test('String#matchAll', assert => {
value: undefined,
done: true,
});
iterator = '1111a2b3cccc'.matchAll(/(\d)(\D)/);
assert.isIterator(iterator);
assert.isIterable(iterator);
assert.deepEqual(iterator.next(), {
value: assign(['1a', '1', 'a'], {
input: '1111a2b3cccc',
index: 3,
}),
done: false,
});
assert.deepEqual(iterator.next(), {
value: undefined,
done: true,
});
assert.throws(() => '1111a2b3cccc'.matchAll(/(\d)(\D)/), TypeError);
iterator = '1111a2b3cccc'.matchAll('(\\d)(\\D)');
assert.isIterator(iterator);
assert.isIterable(iterator);
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/esnext.string.replace-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ QUnit.test('String#replaceAll', assert => {
return 'c';
}), 'aca');
const searcher = {
[Symbol.replaceAll](O, replaceValue) {
[Symbol.replace](O, replaceValue) {
assert.same(this, searcher, '`this` is `searcher`');
assert.same(String(O), 'aba', '`O` is `aba`');
assert.same(String(replaceValue), 'c', '`replaceValue` is `c`');
Expand All @@ -31,7 +31,7 @@ QUnit.test('String#replaceAll', assert => {
assert.throws(() => replaceAll.call(null, 'a', 'b'), TypeError);
assert.throws(() => replaceAll.call(undefined, 'a', 'b'), TypeError);
}
assert.same('b.b.b.b.b'.replaceAll(/\./, 'a'), 'babababab');
assert.throws(() => 'b.b.b.b.b'.replaceAll(/\./, 'a'), TypeError);
assert.same('b.b.b.b.b'.replaceAll(/\./g, 'a'), 'babababab');
const object = {};
assert.same('[object Object]'.replaceAll(object, 'a'), 'a');
Expand Down

0 comments on commit 9ab0f3a

Please sign in to comment.