Skip to content
This repository has been archived by the owner on May 19, 2018. It is now read-only.

Implement nullish coalescing operator in parser #761

Merged
merged 2 commits into from
Oct 14, 2017

Conversation

azz
Copy link
Member

@azz azz commented Oct 14, 2017

Q A
Bug fix? no
Breaking change? no
New feature? yes
Deprecations? no
Spec compliancy? yes
Tests added/pass? yes
Fixed tickets
License MIT

Adding the parser for the nullish coalescing operator, a ?? b, currently at stage 1.

In the AST it isn an operator under BinaryExpression. Should it move to LogicalExpression?

Picked the plugin name: "nullishCoalescingOperator". It's a bit of a mouthful, and may well change.

Ref: babel/proposals#14
Proposal: https://github.com/gisenberg/proposal-null-coalescing

Proposal will likely be renamed from nullary to nullish, see tc39/proposal-nullish-coalescing#3.

I feel like I haven't added enough tests, but I'm not too sure of the edge cases I should test, let me know if you know of any!

Copy link
Member

@hzoo hzoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow amazing work @azz! I'm good with this if the name is good for everyone

@hzoo
Copy link
Member

hzoo commented Oct 14, 2017

For the tests, I would check https://github.com/babel/babylon/pull/742/files which is similar

Could do chained so a ?? b ?? c or multiline, or precedence check

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 fine with plugin name, and agreed on adding the tests @hzoo mentioned

@gisenberg
Copy link
Member

cc @littledan

@hzoo
Copy link
Member

hzoo commented Oct 14, 2017

Fine with merging now if we can get a PR for tests next, (logic shouldn't be any different)

@hzoo hzoo merged commit e53ce38 into babel:master Oct 14, 2017
@azz azz deleted the proposal-nullish-coalescing-operator branch October 15, 2017 00:28
@hzoo
Copy link
Member

hzoo commented Oct 15, 2017

new PR #762, should probably be LogicalExpression instead of BinaryExpression

@@ -791,6 +791,7 @@ enum BinaryOperator {
| "|" | "^" | "&" | "in"
| "instanceof"
| "|>"
| "??"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a LogicalOperator, not a BinaryOperator

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Successfully merging this pull request may close these issues.

5 participants