Add support for the 'y' flag to the RegExp constructor #492

nicolo-ribaudo · 2019-03-07T01:28:13Z

We already partially had this logic in RegExp#@@split: I removed it from there and moved to the RegExp exec internal polyfill.

The changes to the exec constructor are mainly moving things to a single expression to different statements, so that I could place the two SUPPORTS_Y blocks in between.

Fixes #372

zloirock · 2019-03-08T00:32:19Z

IIRC I had some problems with old FF when I played with something like this. Need to test it.

nicolo-ribaudo · 2019-03-08T00:35:07Z

What do you mean approximately with "old"? I can try to download it and check.

zloirock · 2019-03-08T00:36:42Z

I don't remember. Early implementations. It was in 2015.

zloirock · 2019-03-08T00:48:58Z

Most likely it was related this issue.

nicolo-ribaudo · 2019-03-12T23:35:06Z

I have tested this PR in firefox 11: it was affected by that bug which I fixed in 775b17f.

re.sticky can't be set to true, because it is non-configurable and non-enumerble:

EDIT: Fixed

zloirock · 2019-03-12T23:59:42Z

packages/core-js/modules/es.regexp.constructor.js

+      RegExpWrapper
+    );
+
+    if (UNSUPPORTED_Y) defineProperty(result, 'sticky', {


It should be placed on the prototype. I'm not sure that it's 100% safe, but before users issues, we can try it.

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 make sticky a prototype accessor for that property? I don't want to use a weakmap since it would always need to be polyfilled.

For this case, we have https://github.com/zloirock/core-js/blob/master/packages/core-js/internals/internal-state.js

zloirock · 2019-03-13T19:35:41Z

tests/tests/es.regexp.exec.js

@@ -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');


We do not polyfill RegExp constructor in IE8- since we can't add accessors. At least, this test should be ignored in engines without descriptors.

👍

I'm also moving it to a separate file.

nicolo-ribaudo · 2019-03-13T20:34:23Z

I have tested this PR in modern Chrome (full sticky support), Firefox 11 (broken sticky support) and IE11 (no sticky support). Note that in Firefox 11 it is not possible to polyfill the .sticky accessor, so I had to disable some tests there.

nicolo-ribaudo · 2019-03-13T20:36:01Z

packages/core-js-compat/src/data.js

+    edge: '13',
+    firefox: '3',
+    opera: '36',
+    safari: '10.0',


I got this data from MDN. Note that on Firefox 3 the y flags doesn't correctly work with regexp methods, but this compat entry is only about the .sticky accessor.

We don't need data for modern opera - it's generated from chrome.

nicolo-ribaudo · 2019-03-13T20:36:50Z

packages/core-js/internals/fix-regexp-well-known-symbol-logic.js

@@ -20,6 +19,12 @@ var REPLACE_SUPPORTS_NAMED_GROUPS = !fails(function () {
  return ''.replace(re, '$<a>') !== '7';
 });

+// IE <= 11 replaces $0 with the whole match, as if it was $&
+// https://stackoverflow.com/questions/6024666/getting-ie-to-replace-a-regex-with-the-literal-string-0
+var REPLACE_KEEPS_$0 = (function () {


If you prefer I can move this fix to a separate PR. I fixed it here just because I saw the tests failing on IE11 and it is related to RegExps.

Nope, it's fine 👍

zloirock · 2019-03-13T20:47:07Z

tests/compat/tests.js

+      return new RegExp('a', 'y').sticky === true;
+    } catch (e) {
+      return false;
+    }


We don't need try / catch here - we have it in tests runner.

zloirock · 2019-03-13T20:53:28Z

tests/compat/tests.js

@@ -710,6 +710,13 @@ GLOBAL.tests = {
  'es.regexp.flags': function () {


Seems we should also update this and RegExp constructor tests and results, maybe something else.

zloirock · 2019-03-18T21:26:55Z

packages/core-js/modules/es.string.split.js

@@ -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');


zloirock · 2019-03-19T01:06:14Z

packages/core-js/modules/es.string.split.js

 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',


In internalSplit, we have possible RegExp call with y flag.

ThaNarie · 2019-08-10T15:52:56Z

I realise this issue has been stale for a while, but I would just like to add my findings here.

We've been using the fluent library in our project, which heavily relies on the sticky flag, and uses the lastIndex to move the cursor through the string while using multiple regexps with test and exec.
When merging this PR in a core-js fork we used in our project, we ran into 2 major issues, which I think still need to be addressed here:

regexp.exec wasn't included by babel-preset-env because it still has es.regexp.exec as implemented in ie: '9',:

core-js/packages/core-js-compat/src/data.js

Line 792 in 3699c84

ie: '9',

Removing this line properly polyfills the exec for IE.
The sticky flag should also modify the behaviour of regexp.test by moving the lastIndex property the same way as exec does. But since there is no test polyfill included in core-js at all, this behaviour doesn't work in IE.

A quick fix for this is to patch regexp.test with return !!re.exec(str);.

Repro that fails in IE:

var re = new RegExp('\\s+', 'y')
re.test('  aa');
console.log(re.lastIndex); // should be 2, but is 0 in IE with current branch

zloirock · 2019-10-01T14:26:56Z

@ThaNarie since that PR is suspended, feel free to make an alternative version if it's interesting for you.

nicolo-ribaudo · 2019-10-01T15:09:33Z

@ThaNarie I don't have much time to pick this up again, buy if you want you could just take this PR and modify it if it's easier!

HankMcCoy · 2019-11-06T23:50:05Z

My team has been running into the same issues w/ Fluent that @ThaNarie mentioned, so we are planning on taking a shot at getting an updated version of this PR up.

We haven't contributed to core-js before, so we'll dig into the CONTRIBUTING doc and see what we can figure out!

zloirock · 2019-11-06T23:59:11Z

@HankMcCoy if you will have questions, ask me -)

cvle · 2019-12-03T18:46:41Z

@HankMcCoy I will also dedicate some time in the coming weeks. It would be awesome if we could collaborate.

zloirock#492

zloirock#492 Co-authored-by: Nicolò Ribaudo <[email protected]>

zloirock added enhancement es6 labels Mar 8, 2019

Repository owner deleted a comment Mar 11, 2019

nicolo-ribaudo added 3 commits March 12, 2019 23:41

Add support for the 'y' flag to the RegExp constructor

80c19a6

Add semicolon

c571f31

Fix old firefox

775b17f

nicolo-ribaudo force-pushed the re-sticky branch from 77a7102 to 775b17f Compare March 12, 2019 23:31

Remove comment

b8fc64b

Use native "sticky" property in old firefox

5d6a0bd

zloirock reviewed Mar 12, 2019

View reviewed changes

nicolo-ribaudo added 2 commits March 13, 2019 20:11

Move #sticky getter to prototype

4160559

Fix String#replace in IE11

145e4e7

zloirock reviewed Mar 13, 2019

View reviewed changes

Move #sticky to a separate file

06b6d4f

nicolo-ribaudo commented Mar 13, 2019

View reviewed changes

Remove opera

f93b81f

zloirock reviewed Mar 13, 2019

View reviewed changes

Update compat tests

c02cdf3

zloirock reviewed Mar 18, 2019

View reviewed changes

Fix IE8

698daaa

Repository owner deleted a comment from nicolo-ribaudo Mar 18, 2019

zloirock reviewed Mar 19, 2019

View reviewed changes

zloirock force-pushed the master branch from 20ab43c to dcf80f8 Compare May 23, 2019 17:58

zloirock removed this from the 3.1 milestone Jul 1, 2019

cvle added a commit to cvle/core-js that referenced this pull request Dec 16, 2019

Rebase nicolo-ribaudo work

ab567cf

zloirock#492

cvle added a commit to cvle/core-js that referenced this pull request Dec 16, 2019

Rebase nicolo-ribaudo work

05d91a9

zloirock#492 Co-authored-by: Nicolò Ribaudo <[email protected]>

cvle added a commit to cvle/core-js that referenced this pull request Dec 17, 2019

Rebase nicolo-ribaudo work

2934d8e

zloirock#492 Co-authored-by: Nicolò Ribaudo <[email protected]>

cvle mentioned this pull request Dec 17, 2019

Sticky RegExp 'y' and RegExp#test delegation #732

Merged

zloirock closed this in 80d7bfe Dec 17, 2019

nicolo-ribaudo deleted the re-sticky branch December 18, 2019 00:11

snyk-bot mentioned this pull request Jan 8, 2020

[Snyk] Upgrade core-js from 3.3.1 to 3.6.1 tuanicom/incap#31

Merged

snyk-bot mentioned this pull request Jan 30, 2020

[Snyk] Upgrade core-js from 3.2.1 to 3.6.2 preset-io/superset#9

Closed

snyk-bot mentioned this pull request Feb 20, 2020

[Snyk] Upgrade core-js from 3.4.5 to 3.6.4 Narshe1412/Code-Institute-Data-Centric-Project#153

Merged

This was referenced Mar 13, 2020

[Snyk] Upgrade core-js from 3.5.0 to 3.6.4 svoboda-vlad/test-jhipster-upload-from-csv#11

Closed

[Snyk] Upgrade core-js from 3.5.0 to 3.6.4 CodeForBaltimore/Healthcare-Rollcall#43

Merged

[Snyk] Upgrade core-js from 3.4.8 to 3.6.4 ssmbct-netops/CyberChef#1

Open

vaibhavarora14 mentioned this pull request Apr 8, 2020

[Snyk] Upgrade core-js from 3.1.4 to 3.6.4 vaibhavarora14/Banks_List_Demo#14

Merged

snyk-bot mentioned this pull request Apr 25, 2020

[Snyk] Upgrade core-js from 3.0.1 to 3.6.5 rollup-umd/react-styleguidist#4

Open

snyk-bot mentioned this pull request May 5, 2020

[Snyk] Upgrade core-js from 3.6.0 to 3.6.5 MitchTalmadge/CometCrate#30

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for the 'y' flag to the RegExp constructor #492

Add support for the 'y' flag to the RegExp constructor #492

nicolo-ribaudo commented Mar 7, 2019 •

edited

Loading

zloirock commented Mar 8, 2019

nicolo-ribaudo commented Mar 8, 2019

zloirock commented Mar 8, 2019

zloirock commented Mar 8, 2019

nicolo-ribaudo commented Mar 12, 2019 •

edited

Loading

zloirock Mar 12, 2019

nicolo-ribaudo Mar 13, 2019

zloirock Mar 13, 2019

zloirock Mar 13, 2019 •

edited

Loading

nicolo-ribaudo Mar 13, 2019

nicolo-ribaudo commented Mar 13, 2019

nicolo-ribaudo Mar 13, 2019

zloirock Mar 13, 2019

nicolo-ribaudo Mar 13, 2019

zloirock Mar 13, 2019

zloirock Mar 13, 2019

zloirock Mar 13, 2019 •

edited

Loading

zloirock Mar 18, 2019

zloirock Mar 19, 2019

ThaNarie commented Aug 10, 2019

zloirock commented Oct 1, 2019

nicolo-ribaudo commented Oct 1, 2019

HankMcCoy commented Nov 6, 2019

zloirock commented Nov 6, 2019

cvle commented Dec 3, 2019

		@@ -710,6 +710,13 @@ GLOBAL.tests = {
		'es.regexp.flags': function () {

Add support for the 'y' flag to the RegExp constructor #492

Add support for the 'y' flag to the RegExp constructor #492

Conversation

nicolo-ribaudo commented Mar 7, 2019 • edited Loading

zloirock commented Mar 8, 2019

nicolo-ribaudo commented Mar 8, 2019

zloirock commented Mar 8, 2019

zloirock commented Mar 8, 2019

nicolo-ribaudo commented Mar 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zloirock Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zloirock Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThaNarie commented Aug 10, 2019

zloirock commented Oct 1, 2019

nicolo-ribaudo commented Oct 1, 2019

HankMcCoy commented Nov 6, 2019

zloirock commented Nov 6, 2019

cvle commented Dec 3, 2019

nicolo-ribaudo commented Mar 7, 2019 •

edited

Loading

nicolo-ribaudo commented Mar 12, 2019 •

edited

Loading

zloirock Mar 13, 2019 •

edited

Loading

zloirock Mar 13, 2019 •

edited

Loading