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

build(babel): Add optional chaining proposal to babel config #3201

Merged
merged 1 commit into from
Mar 12, 2019

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Mar 12, 2019

overview

As discussed on Slack, this PR adds @babel/plugin-proposal-optional-chaining to our Babel plugin chain. This enables the following syntax:

// before
const foo = bar && bar.baz && bar.baz.qux

// after
const foo = bar?.baz?.qux

Flow is able to type this syntax correctly.

changelog

  • Add optional chaining proposal to babel config

Note: we also discussed adding @babel/plugin-proposal-do-expressions, but Flow does not support this plugin at a syntax level, so we can't move forward (facebook/flow#560).

review requests

I've made one example change in source to demonstrate the kinds of benefits we could see. A couple things to repeat based on our discussions on Slack and in real life:

  • This syntax is easy to overuse. We should be careful and purposeful when we use it, and add comments if anything about it's usage isn't obvious
  • The optional function syntax in this proposal is pretty ugly and a little hard to mentally parse
    • The technical reasoning behind the syntax decisions seems sound, but I still think we should probably avoid using it for function calls

@mcous mcous added app Affects the `app` project chore ready for review protocol designer Affects the `protocol-designer` project components Affects the `components` project labels Mar 12, 2019
@mcous mcous requested review from Kadee80, b-cooper and IanLondon March 12, 2019 17:25
@codecov
Copy link

codecov bot commented Mar 12, 2019

Codecov Report

Merging #3201 into edge will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge    #3201      +/-   ##
==========================================
+ Coverage   53.17%   53.17%   +<.01%     
==========================================
  Files         691      691              
  Lines       20403    20402       -1     
==========================================
  Hits        10849    10849              
+ Misses       9554     9553       -1
Impacted Files Coverage Δ
babel.config.js 0% <ø> (ø) ⬆️
...src/components/AppSettings/AdvancedSettingsCard.js 0% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51846d5...c9ecec8. Read the comment docs.

Copy link
Contributor

@IanLondon IanLondon left a comment

Choose a reason for hiding this comment

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

Riddler

Copy link
Contributor

@Kadee80 Kadee80 left a comment

Choose a reason for hiding this comment

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

❓ 🎊

Copy link
Contributor

@b-cooper b-cooper left a comment

Choose a reason for hiding this comment

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

🚥

@mcous mcous merged commit f1968ba into edge Mar 12, 2019
@mcous mcous deleted the babel-proposals branch March 12, 2019 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Affects the `app` project chore components Affects the `components` project protocol designer Affects the `protocol-designer` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants