Skip to content
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

Optional Chaining Operator #146

Closed
hzoo opened this issue Jan 27, 2017 · 26 comments · Fixed by #204
Closed

Optional Chaining Operator #146

hzoo opened this issue Jan 27, 2017 · 26 comments · Fixed by #204

Comments

@hzoo
Copy link
Contributor

hzoo commented Jan 27, 2017

Stage: 1, Gabriel Isenberg

Slack room at https://babeljs.slack.com/messages/proposal-opt-chaining/ (join at http://slack.babeljs.io/)

Spec Repo: https://github.com/claudepache/es-optional-chaining
TC39 Proposals: https://github.com/tc39/proposals
Slides: https://docs.google.com/presentation/d/11O_wIBBbZgE1bMVRJI8kGnmC6dWCBOwutbN9SWOK0fU/edit#slide=id.p

Figured we should discuss ideas earlier before someone tries to implement it in Babylon?


From the repo:

var street = user.address?.street; // in
var street = user.address && user.address.street; // out
obj?.prop         // optional property access
obj?.[expr]       // ditto
func?.(...args)   // optional function or method call
new C?.(...args)  // optional constructor invocation

In order to allow foo?.3:0 to be parsed as foo ? .3 : 0 rather than foo ?. 3 : 0, a simple lookahead is added at the level of the lexical grammar (the ?. token should not be followed by a decimal digit).

We don’t use the obj?[expr] and func?(...arg) syntax, because of the difficulty for the parser to distinguish those forms from the conditional operator, e.g. obj?[expr].filter(fun):0 and func?(x - 2) + 3 :1.

@mikesherov
Copy link
Contributor

@hzoo, first off, thanks for coming here first. I think the general tendency has been to let y'all do what you want, then ask you to make possible breaking changes to Babylon later, which isn't great.

@RReverser @ariya et. al, do y'all have any interest in helping spec this thing at Stage 1 so we have a better shot of avoiding a breaking change later?

@mikesherov
Copy link
Contributor

@hzoo, second of all, this one seems straightforward to spec.

As a straw man, I propose adding a property named null to NewExpression, MemberExpression, and CallExpression that would be set to true if it was null propagated.

Is that all there is to it? Other bikesheds seem to center around the name of the property: nullPropagation, existential, optional .

Thoughts?

@hzoo
Copy link
Contributor Author

hzoo commented Jan 28, 2017

@mikesherov Yeah it was at least my intention of doing so here first but I forgot for Import since we already had the PR, etc. For all future proposals I will plan to make an ESTree issue first so we can at least discuss and we can always change it again later still. (Our plan is to somehow separately version the proposals from the babylon version). I myself don't want Babel as a project to just "do it's own thing" so thanks for helping us all.

cc @gisenberg @bmeck

@phpnode
Copy link

phpnode commented Jan 30, 2017

bikeshed - the name of the proposal is Optional Chaining, and it returns undefined not null, so optional would seem like a logical name for the property

@bmeck
Copy link

bmeck commented Jan 30, 2017

@phpnode @hzoo @mikesherov The Stage 1 status does not mean we have a proper spec text. Please wait on meeting notes to do anything. Semantics and grammar are still in hot debate. Claude's proposal will most likely not look like the actual spec. In particular the free grouping and always returning undefined are being debated. Await a revised spec based upon what got it to Stage 1 in a few weeks. Even then, I would caution implementing without talking directly to the champion.

@hzoo
Copy link
Contributor Author

hzoo commented Jan 30, 2017

This is just for the AST node format though, not implementation - that probably won't change? (I didn't make a babel issue for the transform plugin, this is just the parser and ESTree just documents the format)

@RReverser
Copy link
Member

@bmeck We put any pre-stage-4 proposals into experimental folder and change them freely, there is nothing scary with that even if spec changes.

@mikesherov
Copy link
Contributor

@bmeck we'll wait on meeting notes, but yeah, once those come out, shouldn't we be trying to land this in Babel so folks can try it out?

@bmeck
Copy link

bmeck commented Jan 30, 2017

@mikesherov the new spec yes, Claude's is the basis but not what was agreed towards stage 1. So, not Claude's.

@mikesherov
Copy link
Contributor

@bmeck can you keep us posted when the notes are available, or if not, @hzoo ?

@hzoo
Copy link
Contributor Author

hzoo commented Feb 6, 2017

aside: @mikesherov I think (if everyone agrees) you can add me / @danez / @loganfsmyth as a member instead of Sebastian since he hasn't been involved in Babel for a while now

@mikesherov
Copy link
Contributor

@hzoo PR please for that

@davidyaha
Copy link

Sorry for dropping in unannounced but I have personal interest at this proposal and I've already contacted @gisenberg on the Babel slack.

@bmeck @hzoo Grammar and semantics of the spec aside, having optional property on MemberExpression and on CallExpression seems like the only sure way to proceed here.

We should maybe leave the NewExpression as it is and add it at later time if the spec will include that.
IMO optional constructor invocation is a bit of an edge case in terms of what the user would like to achieve and whether or not this will encourage bad habits.

@hzoo hzoo changed the title Null Propagation Operator Optional Chaining Operator Jun 16, 2017
@hzoo
Copy link
Contributor Author

hzoo commented Jun 16, 2017

The proposal name is Optional Chaining instead of Null Propagation now and we landed the parser plugin in babylon babel/babylon#545 with

interface MemberExpression <: Expression, Pattern {
  type: "MemberExpression";
  object: Expression | Super;
  property: Expression;
  computed: boolean;
+ optional: boolean | null;
}

interface CallExpression <: Expression {
  type: "CallExpression";
  callee: Expression | Super | Import;
  arguments: [ Expression | SpreadElement ];
+ optional: boolean | null;
}

interface NewExpression <: CallExpression {
  type: "NewExpression";
+ optional: boolean | null;
}

in https://github.com/babel/babylon/releases/tag/v7.0.0-beta.13

@not-an-aardvark
Copy link
Contributor

How does the optional field represent the short-circuiting semantics in the proposal? For example, the semantics of the following expressions are different (proposal notes), but they would have the same AST if we just add an optional field to the a?.b MemberExpression:

a?.b.c // equivalent to `a == null ? void 0 : a.b.c`

(a?.b).c // equivalent to `(a == null ? void 0 : a.b).c`

This syntax seems like it could be a significant problem because ESTree represents the MemberExpression a.b.c identically to (a.b).c. So the semantics of a (something).c MemberExpression could change depending on the presence of an optional field which might be arbitrarily deep in the AST of (something).

@bmeck
Copy link

bmeck commented Feb 13, 2018

I was wrong... skip this @not-an-aardvark your parenthesis are still affecting things, just not to any affect in your example.

a + b + c Is parsed the same as (a + b) + c because the AST is nota Concrete Syntax Tree and is removing grouping by parenthesis by constructing the AST to represent the operations rather than the syntax used to construct them. It does not follow left to right ordering etc.

The above parses to:

{
    "type": "Program",
    "body": [
        {
            "type": "ExpressionStatement",
            "expression": {
                "type": "BinaryExpression",
                "operator": "+",
                "left": {
                    "type": "BinaryExpression",
                    "operator": "+",
                    "left": {
                        "type": "Identifier",
                        "name": "a"
                    },
                    "right": {
                        "type": "Identifier",
                        "name": "b"
                    }
                },
                "right": {
                    "type": "Identifier",
                    "name": "c"
                }
            }
        }
    ],
    "sourceType": "script"
}

But moving the parenthesis around to be a+(b+c) you can see that parenthesis are significant:

{
    "type": "Program",
    "body": [
        {
            "type": "ExpressionStatement",
            "expression": {
                "type": "BinaryExpression",
                "operator": "+",
                "left": {
                    "type": "Identifier",
                    "name": "a"
                },
                "right": {
                    "type": "BinaryExpression",
                    "operator": "+",
                    "left": {
                        "type": "Identifier",
                        "name": "b"
                    },
                    "right": {
                        "type": "Identifier",
                        "name": "c"
                    }
                }
            }
        }
    ],
    "sourceType": "script"
}

This leads to your question. The optional property can be applied to the proper operation rather than the notion of parenthesis and grouping. Just like the first example, the inner most Node is going to set the optional flag for (a?.b).c on the member expression for a?.b which would be evaluated before the expression for .c.

@loganfsmyth
Copy link
Contributor

@bmeck The optional boolean is not quite enough because the optional flag being on an inner expression doesn't differentiate between (a?.b).c and a?.b.c because optional: true means that it uses ?., but it doesn't have enough information to know where in the chain the behavior is supposed to break.

@not-an-aardvark Babel is currently thinking of exposing this as a new OptionalMemberExpression node, with the expectation that the object of that would always either be a MemberExpression with optional: true set, or be another OptionalMemberExpression node. The spec no longer allows optional new and it doesn't allow tagged template literals to be used in chained expressions, so a single new node type along with the boolean flag should be enough to represent everything.

@bmeck
Copy link

bmeck commented Feb 13, 2018

oh, I misunderstood the situation.

@not-an-aardvark
Copy link
Contributor

Is it necessary to have a distinction between "MemberExpression with optional:true " and "OptionalMemberExpression"? It seems like in the expression (a?.b.c).d, it would be sufficient to represent the .b, and .c accesses as part of an OptionalMemberExpression, and represent the .d access as part of a normal MemberExpression. In other words, would it be possible to make the ASTs identical for a?.b.c and a?.b?.c ?

@loganfsmyth
Copy link
Contributor

would it be possible to make the ASTs identical for a?.b.c and a?.b?.c

Those two have different runtime behavior once a.b is successfully loaded, so I don't think that would be doable.

@not-an-aardvark
Copy link
Contributor

Good point, I had forgotten about that distinction.

I wonder if there is a better way to semantically represent the distinction --
it would be difficult to guess the meanings of "OptionalMemberExpression" and "MemberExpression with optional: true" since they both use the word "optional" to mean slightly different things.

For example, maybe a shortCircuit: true flag could be used to represent a MemberExpression which can get short-circuited (i.e. the same as the OptionalMemberExpression node in this discussion), and an optional: true flag could be used to represent a MemberExpression which uses the ?. operator. Then (a?.b.c).d would be parsed to:

{
  "type": "MemberExpression",
  "object": {
    "type": "MemberExpression",
    "object": {
      "type": "MemberExpression",
      "object": {
        "type": "Identifier",
        "name": "a"
      },
      "property": {
        "type": "Identifier",
        "name": "b"
      },
      "optional": true,
      "shortCircuit": false
    },
    "property": {
      "type": "Identifier",
      "name": "c"
    },
    "optional": false,
    "shortCircuit": true
  },
  "property": {
    "type": "Identifier",
    "name": "d"
  },
  "optional": false,
  "shortCircuit": false
}

@loganfsmyth
Copy link
Contributor

loganfsmyth commented Feb 14, 2018

I wonder if there is a better way to semantically represent the distinction --
it would be difficult to guess the meanings of "OptionalMemberExpression" and "MemberExpression with optional: true" since they both use the word "optional" to mean slightly different things.

I agree. We had a bit of discussion in babel/babel#7256 (comment) and essentially decided that we didn't want to block the person exploring the implementation. I'm still totally open to choosing a clearer name.

cc @jridgewell @nveenjain

Thoughts on shortCircuit: boolean?

@jridgewell
Copy link

I'm a bit confused by the discussion. MemberExpression can't have an optional (renaming this is totally possible), only OptionalMemberExpression. Taking (a?.b.c).d:

{
  "type": "MemberExpression",
  "object": {
    "type": "OptionalMemberExpression",
    "object": {
      "type": "OptionalMemberExpression",
      "object": {
        "type": "Identifier",
        "name": "a"
      },
      "property": {
        "type": "Identifier",
        "name": "b"
      },
      "optionalOrShortCircuitBikeShed": true,
    },
    "property": {
      "type": "Identifier",
      "name": "c"
    },
    "optionalOrShortCircuitBikeShed": false,
  },
  "property": {
    "type": "Identifier",
    "name": "d"
  },
}

Once you get a ?., it creates a OptionalMemberExpression with optional: true. Every . after that continues to create OptionalMemberExpressions with optional: false. If you hit another ?., its optional: true again, and continues. Once you hit a parenthesis, it breaks the chain and further .s create regular MemberExpressions.

Note that shortCircuit doesn't quite capture the optional use (it's actually the inverse). root was floated.

@loganfsmyth
Copy link
Contributor

Sorry you're totally right, I misstated which node type has the boolean flag.

@nveenjain
Copy link

Yes, I am open for suggestions for other names, but currently optional seems best choice of the three proposed.

facebook-github-bot pushed a commit to facebook/flow that referenced this issue Apr 19, 2018
Summary:
D7029173 implemented basic lexer/parser support for optional chaining, based on the [Babylon implementation](babel/babylon#545) linked from the [proposal](https://github.com/tc39/proposal-optional-chaining#related-issues). However, that code is out of sync with the current specification. Babel has since [updated](babel/babel#7256) its code to fix those issues. This diff parallels those changes, which should make their way into [ESTree](estree/estree#146).

Notes:
* We never prohibited `delete`-ing an optional chain, so there's no need to add this back in.
* D7029173 already prohibits combining optional chains with `super`, `new` expressions, and template literals.
* Rather than annotating `Member` and `Call` AST nodes with an `optional` property, we have new `OptionalMember` and `OptionalCall` nodes. This makes detecting and tracking optional chains easier, and we can leverage this to track where [parentheses have curtailed short-circuiting](https://github.com/tc39/proposal-optional-chaining#edge-case-grouping).

Reviewed By: mroch

Differential Revision: D7380919

fbshipit-source-id: bbfaa620936bd14c7f8e06a85ae22d81b144a6d4
fishythefish pushed a commit to facebook/flow that referenced this issue Apr 25, 2018
Summary:
D7029173 implemented basic lexer/parser support for optional chaining, based on the [Babylon implementation](babel/babylon#545) linked from the [proposal](https://github.com/tc39/proposal-optional-chaining#related-issues). However, that code is out of sync with the current specification. Babel has since [updated](babel/babel#7256) its code to fix those issues. This diff parallels those changes, which should make their way into [ESTree](estree/estree#146).

Notes:
* We never prohibited `delete`-ing an optional chain, so there's no need to add this back in.
* D7029173 already prohibits combining optional chains with `super`, `new` expressions, and template literals.
* Rather than annotating `Member` and `Call` AST nodes with an `optional` property, we have new `OptionalMember` and `OptionalCall` nodes. This makes detecting and tracking optional chains easier, and we can leverage this to track where [parentheses have curtailed short-circuiting](https://github.com/tc39/proposal-optional-chaining#edge-case-grouping).

Reviewed By: mroch

Differential Revision: D7380919

fbshipit-source-id: 9a662b37dbac7b36262fdc07681f323f22c75c8f
@devsnek
Copy link

devsnek commented Aug 12, 2019

While implementing optional chaining in engine262 I found that babel's approach, while excellent for generating jump tables (which is what babel does), is not a fantastic representation of the actual structure of an optional chain in terms of associativity.

Tentatively I submit this structure, although the names and such are obviously up for bikeshedding: https://github.com/engine262/engine262/blob/e4c6798e1f3f89505f4c23eecc9e5d647af6b00e/src/parse.mjs#L84-L192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.