-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for the 'y' flag to the RegExp constructor #492
Changes from 5 commits
80c19a6
c571f31
775b17f
b8fc64b
5d6a0bd
4160559
145e4e7
06b6d4f
f93b81f
c02cdf3
698daaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
'use strict'; | ||
|
||
var fails = require('./fails'); | ||
|
||
// babel-minify transpiles RegExp('a', 'y') -> /a/y and it causes SyntaxError, | ||
// so we use an intermediate function. | ||
function RE(s, f) { | ||
return RegExp(s, f); | ||
} | ||
|
||
exports.UNSUPPORTED_Y = fails(function () { | ||
// babel-minify transpiles RegExp('a', 'y') -> /a/y and it causes SyntaxError | ||
var re = RE('a', 'y'); | ||
re.lastIndex = 2; | ||
return re.exec('abcd') != null; | ||
}); | ||
|
||
exports.BROKEN_CARET = fails(function () { | ||
// https://bugzilla.mozilla.org/show_bug.cgi?id=773687 | ||
var re = RE('^r', 'gy'); | ||
re.lastIndex = 2; | ||
return re.exec('str') != null; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,14 +8,10 @@ var advanceStringIndex = require('../internals/advance-string-index'); | |
var toLength = require('../internals/to-length'); | ||
var callRegExpExec = require('../internals/regexp-exec-abstract'); | ||
var regexpExec = require('../internals/regexp-exec'); | ||
var fails = require('../internals/fails'); | ||
var arrayPush = [].push; | ||
var min = Math.min; | ||
var MAX_UINT32 = 0xffffffff; | ||
|
||
// babel-minify transpiles RegExp('x', 'y') -> /x/y and it causes SyntaxError | ||
var SUPPORTS_Y = !fails(function () { return !RegExp(MAX_UINT32, 'y'); }); | ||
|
||
// @@split logic | ||
require('../internals/fix-regexp-well-known-symbol-logic')( | ||
'split', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In |
||
|
@@ -99,24 +95,22 @@ require('../internals/fix-regexp-well-known-symbol-logic')( | |
var flags = (rx.ignoreCase ? 'i' : '') + | ||
(rx.multiline ? 'm' : '') + | ||
(rx.unicode ? 'u' : '') + | ||
(SUPPORTS_Y ? 'y' : 'g'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ! |
||
'y'; | ||
|
||
// ^(? + rx + ) is needed, in combination with some S slicing, to | ||
// simulate the 'y' flag. | ||
var splitter = new C(SUPPORTS_Y ? rx : '^(?:' + rx.source + ')', flags); | ||
var splitter = new C(rx, flags); | ||
var lim = limit === undefined ? MAX_UINT32 : limit >>> 0; | ||
if (lim === 0) return []; | ||
if (S.length === 0) return callRegExpExec(splitter, S) === null ? [S] : []; | ||
var p = 0; | ||
var q = 0; | ||
var A = []; | ||
while (q < S.length) { | ||
splitter.lastIndex = SUPPORTS_Y ? q : 0; | ||
var z = callRegExpExec(splitter, SUPPORTS_Y ? S : S.slice(q)); | ||
splitter.lastIndex = q; | ||
var z = callRegExpExec(splitter, S); | ||
var e; | ||
if ( | ||
z === null || | ||
(e = min(toLength(splitter.lastIndex + (SUPPORTS_Y ? 0 : q)), S.length)) === p | ||
(e = min(toLength(splitter.lastIndex), S.length)) === p | ||
) { | ||
q = advanceStringIndex(S, q, unicodeMatching); | ||
} else { | ||
|
@@ -133,6 +127,5 @@ require('../internals/fix-regexp-well-known-symbol-logic')( | |
return A; | ||
} | ||
]; | ||
}, | ||
!SUPPORTS_Y | ||
} | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,3 +26,33 @@ QUnit.test('RegExp#exec capturing groups', assert => { | |
// #replace, but here also #replace is buggy :( | ||
// assert.deepEqual(/(a?)?/.exec('x'), ['', undefined], '/(a?)?/.exec("x") returns ["", undefined]'); | ||
}); | ||
|
||
QUnit.test('RegExp#exec sticky', assert => { | ||
const re = new RegExp('a', 'y'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not polyfill There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 I'm also moving it to a separate file. |
||
const str = 'bbabaab'; | ||
assert.strictEqual(re.lastIndex, 0, '#1'); | ||
|
||
assert.strictEqual(re.exec(str), null, '#2'); | ||
assert.strictEqual(re.lastIndex, 0, '#3'); | ||
|
||
re.lastIndex = 1; | ||
assert.strictEqual(re.exec(str), null, '#4'); | ||
assert.strictEqual(re.lastIndex, 0, '#5'); | ||
|
||
re.lastIndex = 2; | ||
assert.deepEqual(re.exec(str), ['a'], '#6'); | ||
assert.strictEqual(re.lastIndex, 3, '#7'); | ||
|
||
assert.strictEqual(re.exec(str), null, '#8'); | ||
assert.strictEqual(re.lastIndex, 0, '#9'); | ||
|
||
re.lastIndex = 4; | ||
assert.deepEqual(re.exec(str), ['a'], '#10'); | ||
assert.strictEqual(re.lastIndex, 5, '#11'); | ||
|
||
assert.deepEqual(re.exec(str), ['a'], '#12'); | ||
assert.strictEqual(re.lastIndex, 6, '#13'); | ||
|
||
assert.strictEqual(re.exec(str), null, '#14'); | ||
assert.strictEqual(re.lastIndex, 0, '#15'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be placed on the prototype. I'm not sure that it's 100% safe, but before users issues, we can try it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we place it in the prototype, I'm not sure how we could check if the regexp is sticky.
Maybe I could add a
__core-js_sticky__
own property and then makesticky
a prototype accessor for that property? I don't want to use a weakmap since it would always need to be polyfilled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this case, we have https://github.com/zloirock/core-js/blob/master/packages/core-js/internals/internal-state.js