Parenthesized expressions by arv · Pull Request #8025 · babel/babel

arv · 2018-05-24T05:48:07Z

Q	A
Fixed Issues?	Fixes #7661, closes #7015
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?	New feature
Tests Added + Pass?	Yes
Documentation PR
Any Dependency Changes?	No
License	MIT

This adds a parser option called createParenthesizedExpressions, which when enabled creates ParenthesizedExpression nodes instead of setting extra.parenthesized.

nicolo-ribaudo · 2018-05-24T09:19:25Z

I think that this shouldn't be a Babylon plugin (since plugins are meant to enable new syntax), but it should just be an option (which you already implemented).
Can you add a test about (foo) => {} works with this new option enabled?

arv · 2018-05-25T19:53:25Z

All done!

Removed the parser plugin
Added more tests and fixed/improved issues related to lval.

babel-bot · 2018-05-25T20:53:45Z

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

babel-bot · 2018-05-25T20:53:45Z

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

arv · 2018-05-25T21:08:04Z

packages/babel-generator/test/fixtures/comments/create-parenthesized-expressions/options.json

@@ -0,0 +1,3 @@
+{
+  "parserOpts": {"createParenthesizedExpressions": true}


This works because I updated the runner in packages/babel-generator/test/index.js to spread the parserOpts into the parser options.

arv · 2018-05-30T03:58:25Z

Rebased on top of 2a8ebbe

arv · 2018-11-06T22:09:16Z

I'm still interested in getting this feature in. We are using Babel more and more in our tools and using a fork is getting old. Any chance of this getting another review?

@hzoo @nicolo-ribaudo

arv · 2019-02-19T19:50:11Z

This is still blocking us. Any suggestions on how to proceed?

CC @rafaelweinstein

jridgewell · 2019-02-19T19:59:21Z

AMP Project also needed this. We solved it by running a custom plugin as the first step in our babel config:

// https://github.com/ampproject/amphtml/blob/2dc43d8df8a899f95348c2149aceddc9ca96b5a0/build-system/babel-plugins/babel-plugin-transform-parenthesize-expression/index.js
module.exports = function(babel) {
  const {types: t} = babel;
  return {
    visitor: {
      Expression: {
        exit(path) {
          const {node} = path;
          const {parenthesized} = node.extra || {};
          if (!parenthesized) {
            return;
          }

          path.replaceWith(t.parenthesizedExpression(node));
          path.skip();
        },
      },
    },
  };
};

A babel-parser config option would be welcome.

nicolo-ribaudo

Apart from some minor comments, this PR looks pretty good.

nicolo-ribaudo · 2019-02-19T20:15:40Z

packages/babel-parser/src/parser/expression.js

-    return val;
+    const parenExpression = this.startNodeAt(startPos, startLoc);
+    parenExpression.expression = val;
+    this.finishNodeAt(


Isn't using this.finishNode enough? The end position should be the correct one at this point I think.

nicolo-ribaudo · 2019-02-19T20:16:25Z

packages/babel-parser/test/fixtures/core/uncategorised/556/input.js

@@ -0,0 +1 @@
+({x}) = {x: 1};


Could you also test ([ a ]) = []?

packages/babel-parser/src/parser/statement.js

arv

resolved the merge conflicts
Use finishNode instead of finishNodeAt
add test for ([a]) = []
remove dead code (parseParenExpression)

packages/babel-parser/src/parser/statement.js

arv · 2019-02-20T06:17:33Z

packages/babel-parser/test/fixtures/core/uncategorised/556/input.js

@@ -0,0 +1 @@
+({x}) = {x: 1};


nicolo-ribaudo · 2019-02-20T10:00:44Z

packages/babel-parser/src/options.js

  strictMode: ?boolean,
  ranges: boolean,
  tokens: boolean,
+  createParenthesizedExpressions: boolean,


This also needs to be added to babel-parser/typings/index.d.ts

I assume you mean packages/babel-parser/typings/babel-parser.d.ts

arv · 2019-02-20T21:13:15Z

Thanks @danez

existentialism

👍

When set to `true` we create `ParenthesizedExpression` nodes instead of setting `extra.parenthesized`.

nicolo-ribaudo · 2019-02-23T08:02:38Z

We were waiting to decide to release v7.4 😛

jridgewell · 2019-02-23T09:47:22Z

Whoops, didn't see the milestone.

This reverts commit dd8b700.

* Add parser createParenthesizedExpressions option … When set to `true` we create `ParenthesizedExpression` nodes instead of setting `extra.parenthesized`. * Also update babel-parser.d.ts

arv force-pushed the parenthesized-expressions branch 4 times, most recently from dfe4825 to eff42c6 Compare May 24, 2018 05:54

nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: parser labels May 24, 2018

arv force-pushed the parenthesized-expressions branch from eff42c6 to 8e9629a Compare May 25, 2018 19:47

arv commented May 25, 2018

View reviewed changes

arv force-pushed the parenthesized-expressions branch from 8e9629a to e1b1b23 Compare May 30, 2018 03:54

arv force-pushed the parenthesized-expressions branch from e1b1b23 to f77818a Compare November 6, 2018 22:06

nicolo-ribaudo self-requested a review February 19, 2019 19:55

nicolo-ribaudo reviewed Feb 19, 2019

View reviewed changes

danez self-requested a review February 19, 2019 22:02

danez reviewed Feb 19, 2019

View reviewed changes

packages/babel-parser/src/parser/statement.js Show resolved Hide resolved

nicolo-ribaudo mentioned this pull request Feb 19, 2019

[breaking] Remove TSParenthesizedType unless createParenthesizedExpressions is enabled #9546

Closed

arv force-pushed the parenthesized-expressions branch from f77818a to 75759f3 Compare February 20, 2019 06:19

arv commented Feb 20, 2019

View reviewed changes

nicolo-ribaudo reviewed Feb 20, 2019

View reviewed changes

danez approved these changes Feb 20, 2019

View reviewed changes

nicolo-ribaudo approved these changes Feb 20, 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 Feb 20, 2019

nicolo-ribaudo added this to the v7.4.0 milestone Feb 20, 2019

existentialism approved these changes Feb 21, 2019

View reviewed changes

arv added 2 commits February 23, 2019 02:28

Add parser createParenthesizedExpressions option …

06c7649

When set to `true` we create `ParenthesizedExpression` nodes instead of setting `extra.parenthesized`.

Also update babel-parser.d.ts

88ed72b

jridgewell force-pushed the parenthesized-expressions branch from 3f2e477 to 88ed72b Compare February 23, 2019 07:35

jridgewell approved these changes Feb 23, 2019

View reviewed changes

jridgewell merged commit dd8b700 into babel:master Feb 23, 2019

jridgewell added a commit that referenced this pull request Feb 23, 2019

Revert "Parenthesized expressions (#8025)"

3e78770

This reverts commit dd8b700.

jridgewell mentioned this pull request Feb 23, 2019

Revert "Parenthesized expressions" #9569

Merged

nicolo-ribaudo pushed a commit that referenced this pull request Feb 25, 2019

Revert "Parenthesized expressions (#8025)"

fc1ea7f

This reverts commit dd8b700.

nicolo-ribaudo mentioned this pull request Feb 25, 2019

Parenthesized expressions (#8025) #9581

Merged

nicolo-ribaudo removed the PR: New Feature 🚀 A type of pull request used for our changelog categories label Feb 25, 2019

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019

		@@ -0,0 +1,3 @@
		{
		"parserOpts": {"createParenthesizedExpressions": true}

Uh oh!

Conversation

arv commented May 24, 2018 • edited by nicolo-ribaudo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo commented May 24, 2018

Uh oh!

arv commented May 25, 2018

Uh oh!

babel-bot commented May 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented May 25, 2018

Uh oh!

arv May 25, 2018

Choose a reason for hiding this comment

Uh oh!

arv commented May 30, 2018

Uh oh!

arv commented Nov 6, 2018

Uh oh!

arv commented Feb 19, 2019

Uh oh!

jridgewell commented Feb 19, 2019

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

arv Feb 20, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arv Feb 20, 2019

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Feb 20, 2019

Choose a reason for hiding this comment

Uh oh!

arv Feb 20, 2019

Choose a reason for hiding this comment

Uh oh!

arv commented Feb 20, 2019

Uh oh!

existentialism left a comment

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo commented Feb 23, 2019

Uh oh!

jridgewell commented Feb 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

arv commented May 24, 2018 •

edited by nicolo-ribaudo

Loading

babel-bot commented May 25, 2018 •

edited

Loading