Merge multiple regex transform plugin by JLHwung · Pull Request #10447 · babel/babel

JLHwung · 2019-09-16T22:35:56Z

Q	A
Fixed Issues?	Fixes #8951, closes #10347, fixes #10367, fixes #9892, fixes #9199, fixes #10601
Patch: Bug Fix?	Yes
Tests Added + Pass?	Yes
Any Dependency Changes?	Yes
License	MIT

This PR is includes refactor on #10430. I will rebase once it gets merged.

In this PR we merge multiple regex transformers into a single meta plugin so that regex transform can be finished in one-pass. The current modular transform plugin is essentially turned into transformer switches. By doing this we provide support for multiple ESNext regex features
such as /(?<name>\p{Unified_Ideograph}{3}/u.

We also set lookbehind: true to regexpu-core so that our regex transformer can always parse the lookbehind assertions. I don't think we require babel-syntax-regex-lookahead here as we are not parsing every single regex and lookbehind: true is only meaningful when user used it with other transformable features.

Note that although it is a bug fix, I think we may target to a 7.7.0 release as 1) it relies on upstream minor bump. 2) it introduces new package.

Todo:

prevent regexpu from rewritting symbol-kind -: fix: skip ascii symbol in process mathiasbynens/regexpu-core#31

Edits: I realize that in practice there is no browser that supports property escape but not named capturing groups, as they are supported in same version, we do not rely on mathiasbynens/regexpu-core#30 anymore.

babel-bot · 2019-09-16T22:43:39Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11623/

...in-transform-named-capturing-groups-regex/test/fixtures/wrapper/looks-like-a-group/output.js

buildsize · 2019-09-17T20:11:17Z

File name	Previous Size	New Size	Change
babel-preset-env.js	2.69 MB	2.74 MB	57.07 KB (2%)
babel-preset-env.min.js	1.63 MB	1.66 MB	26.54 KB (2%)
babel.js	2.9 MB	2.95 MB	51.57 KB (2%)
babel.min.js	1.6 MB	1.63 MB	24.39 KB (1%)

nicolo-ribaudo · 2019-09-19T21:17:22Z

packages/babel-helper-create-regexp-features-plugin/src/util.js

+  const flagsIncludesU = flags.includes("u");
+
+  if (flagsIncludesU === true) {
+    if (hasFeature(features, FEATURES.unicodeFlag) === false) {


Nit: Don't you like !? 😛
Also all the === trues are unnecessary.

Actually I have no preference on ! or === false until I read the compiled instructions:

This is what happens under the hood when we write if(a) { // blablabla }

0x1fafde0d1bf2 32 4c8b4510 REX.W movq r8,[rbp+0x10] 0x1fafde0d1bf6 36 41f6c001 testb r8,0x1 0x1fafde0d1bfa 3a 0f84cf140000 jz 0x1fafde0d30cf <+0x150f> -- B4 start -- 0x1fafde0d1c00 40 4d3945f8 REX.W cmpq [r13-0x8] (root (false_value)),r8 0x1fafde0d1c04 44 0f8438000000 jz 0x1fafde0d1c42 <+0x82> -- B5 start -- 0x1fafde0d1c0a 4a 4d394500 REX.W cmpq [r13+0x0] (root (empty_string)),r8 0x1fafde0d1c0e 4e 0f842e000000 jz 0x1fafde0d1c42 <+0x82> -- B6 start -- 0x1fafde0d1c14 54 4d8b48ff REX.W movq r9,[r8-0x1] 0x1fafde0d1c18 58 41f6410d10 testb [r9+0xd],0x10 0x1fafde0d1c1d 5d 0f851f000000 jnz 0x1fafde0d1c42 <+0x82> -- B7 start -- -- B8 start -- 0x1fafde0d1c23 63 4d398d80000000 REX.W cmpq [r13+0x80] (root (heap_number_map)),r9 0x1fafde0d1c2a 6a 0f8486140000 jz 0x1fafde0d30b6 <+0x14f6> -- B9 start -- 0x1fafde0d1c30 70 4d398d40010000 REX.W cmpq [r13+0x140] (root (bigint_map)),r9 0x1fafde0d1c37 77 0f8466140000 jz 0x1fafde0d30a3 <+0x14e3> 0x1fafde0d1c3d 7d e913000000 jmp 0x1fafde0d1c55 <+0x95>

Basically it compares a to these four objects: false, "", 0 and 0n. Since JavaScript is weak-typed, even if you know for sure a could only be a boolean, the compiler would not simplify if(a) to the following codes, which is simplified from if(a === true).

0x1fafde0d1bf2 32 4c8b4510 REX.W movq r8,[rbp+0x10] 0x1fafde0d1bf6 36 41f6c001 testb r8,0x1 0x1fafde0d1bfa 3a 0f84cf140000 jz 0x1fafde0d30cf <+0x150f> -- B4 start -- 0x1fafde0d1c00 40 4d3945f8 REX.W cmpq [r13-0x8] (root (false_value)),r8 0x1fafde0d1c04 44 0f8438000000 jz 0x1fafde0d1c42 <+0x82>

However, these branch instructions would never be our performance bottleneck thanks to complex branch prediction in our modern CPU. I am happy to rewrite into more common code style as long as you find out. 😀

packages/babel-helper-create-regexp-features-plugin/src/util.js

packages/babel-helper-create-regexp-features-plugin/src/index.js

packages/babel-helper-create-regexp-features-plugin/src/features.js

existentialism · 2019-10-02T02:37:09Z

packages/babel-helper-create-regexp-features-plugin/src/index.js

+//       as 70000100005. This method is easier than using a semver-parsing
+//       package, but it breaks if we release x.y.z where x, y or z are
+//       greater than 99_999.
+const version = pkg.version.split(".").reduce((v, x) => v * 1e5 + +x, 0);


FWIW, I think it's perfectly fine using semver to parse (since we use it elsewhere)

It's my fault 😛 I used this logic for class features

I think it is okay to leave it as is because all we need is a version sorter and we are not using semver elsewhere.

packages/babel-plugin-proposal-unicode-property-regex/package.json

existentialism · 2019-10-02T02:39:19Z

packages/babel-plugin-proposal-unicode-property-regex/src/index.js

  }

-  return {
+  return createRegExpFeaturePlugin({


💯, clean!

existentialism

Great work!

JLHwung · 2019-10-24T15:15:54Z

@existentialism @nicolo-ribaudo A new fix 7fbd168 has been pushed since you reviewed. In this fix we apply dotAllFlag when flags includes u and we have to apply rewritePattern.

Note that this approach is not ideal. I will propose a useDotAllFlag to regexpu-core so we can preserve the s flag if babel is not instructed to transform them. And we can expect subsequent fixes coming in v7.7.x.

peey · 2019-11-08T20:08:15Z

packages/babel-helper-create-regexp-features-plugin/README.md

+
+> Compile ESNext Regular Expressions to ES5
+
+See our website [@babel/helper-create-regexp-features-plugin](https://babeljs.io/docs/en/next/babel-helper-create-regexp-features-plugin.html) for more information.


This link is dead. Is there supposed to be anything here?

JLHwung mentioned this pull request Sep 16, 2019

breaking: decouple parser switch and transformer switch mathiasbynens/regexpu-core#30

Merged

JLHwung commented Sep 16, 2019

View reviewed changes

...in-transform-named-capturing-groups-regex/test/fixtures/wrapper/looks-like-a-group/output.js Show resolved Hide resolved

nicolo-ribaudo self-requested a review September 16, 2019 23:13

nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Sep 16, 2019

JLHwung force-pushed the helper-create-regexp-features-plugin branch from 71816cb to ba51deb Compare September 18, 2019 02:16

JLHwung marked this pull request as ready for review September 19, 2019 16:56

nicolo-ribaudo added this to the v7.7.0 milestone Sep 19, 2019

nicolo-ribaudo reviewed Sep 19, 2019

View reviewed changes

JLHwung force-pushed the helper-create-regexp-features-plugin branch 2 times, most recently from ffa7989 to db67055 Compare September 19, 2019 23:39

nicolo-ribaudo approved these changes Oct 1, 2019

View reviewed changes

existentialism reviewed Oct 2, 2019

View reviewed changes

packages/babel-helper-create-regexp-features-plugin/src/features.js Outdated Show resolved Hide resolved

existentialism reviewed Oct 2, 2019

View reviewed changes

packages/babel-plugin-proposal-unicode-property-regex/package.json Outdated Show resolved Hide resolved

existentialism reviewed Oct 2, 2019

View reviewed changes

existentialism approved these changes Oct 2, 2019

View reviewed changes

nicolo-ribaudo added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Oct 3, 2019

nicolo-ribaudo mentioned this pull request Oct 24, 2019

Regression in @babel/plugin-transform-named-capturing-groups-regex with unicode regexps #10601

Closed

JLHwung added 10 commits October 24, 2019 10:14

feat: implement create-regexp-features-plugin

00e0312

fix: test input is not effective

04b48fe

refactor: leverage create-regexp-features-plugin

0812de6

test: add more test cases

9a714cf

test: update test fixture

5a2415f

chore: add type annotation to features

a1e0413

test: add regression test for issue 9892

c019c03

add regression test for issue 9199

60f9179

address review comments from Nicolò

6e8f1bd

address review comments from Brian

bcc46c7

JLHwung added 2 commits October 24, 2019 11:01

small tweaks

c65a129

Enable dotAllFlag when flags includes u

7fbd168

JLHwung force-pushed the helper-create-regexp-features-plugin branch from e8b960c to 7fbd168 Compare October 24, 2019 15:10

nicolo-ribaudo self-requested a review October 24, 2019 15:16

nicolo-ribaudo approved these changes Oct 24, 2019

View reviewed changes

existentialism approved these changes Oct 24, 2019

View reviewed changes

nicolo-ribaudo merged commit 8ffca04 into babel:master Oct 29, 2019

peey reviewed Nov 8, 2019

View reviewed changes

snyk-bot mentioned this pull request Nov 14, 2019

[Snyk] Upgrade @babel/runtime from 7.6.3 to 7.7.1 cds-snc/report-a-cybercrime#936

Merged

This was referenced Nov 28, 2019

[Snyk] Upgrade @babel/runtime from 7.0.0-beta.42 to 7.7.2 ali8889/mist#4

Open

[Snyk] Upgrade @babel/runtime from 7.5.5 to 7.7.4 dyna-dot/CyberChef#3

Open

This was referenced Dec 5, 2019

[Snyk] Upgrade @babel/runtime from 7.3.1 to 7.7.4 dyna-dot/bit-javascript#5

Open

[Snyk] Upgrade @babel/runtime from 7.5.5 to 7.7.4 jakeherp/todo-list#1

Merged

snyk-bot mentioned this pull request Jan 12, 2020

[Snyk] Upgrade @babel/runtime from 7.4.4 to 7.7.7 mboocannon/msgraph-sdk-javascript#1

Open

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 8, 2020

lock bot locked as resolved and limited conversation to collaborators Feb 8, 2020


		> Compile ESNext Regular Expressions to ES5

		See our website [@babel/helper-create-regexp-features-plugin](https://babeljs.io/docs/en/next/babel-helper-create-regexp-features-plugin.html) for more information.

Uh oh!

Conversation

JLHwung commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

buildsize bot commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo Sep 19, 2019

Choose a reason for hiding this comment

Uh oh!

JLHwung Sep 19, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

existentialism Oct 2, 2019

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

existentialism Oct 2, 2019

Choose a reason for hiding this comment

Uh oh!

existentialism left a comment

Choose a reason for hiding this comment

Uh oh!

JLHwung commented Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peey Nov 8, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JLHwung commented Sep 16, 2019 •

edited

Loading

babel-bot commented Sep 16, 2019 •

edited

Loading

buildsize bot commented Sep 17, 2019 •

edited

Loading

nicolo-ribaudo Oct 2, 2019 •

edited

Loading

JLHwung commented Oct 24, 2019 •

edited

Loading