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): Upgrade babel to v7 #2858

Merged
merged 1 commit into from
Jan 7, 2019
Merged

build(babel): Upgrade babel to v7 #2858

merged 1 commit into from
Jan 7, 2019

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Jan 3, 2019

overview

Chore PR to upgrade babel to v7 (as discussed in #2815 and #2568). Key benefits:

  • Sane and consolidated config file with support for per-project overrides (see babel.config.js)
  • Support for <> instead of writing out <React.Fragment>
  • Eases exploration of (gradually) migrating from Flow to TypeScript via Babel if and only if that's a thing that would benefit us

changelog

  • Upgrade to Babel v7
  • Added circular dependency plugin to webpack
    • Build project with ANALYZER=true or ANALYZER=1 to turn on the bundle analyzer and circular dependency checking

review requests

Make sure our Flow-based projects still work:

  • app
  • app-shell
  • components
    • discovery-client
  • labware-designer
  • protocol-designer
  • protocol-library-kludge
  • shared-data

@codecov
Copy link

codecov bot commented Jan 4, 2019

Codecov Report

Merging #2858 into edge will increase coverage by 2.3%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             edge    #2858     +/-   ##
=========================================
+ Coverage   51.21%   53.51%   +2.3%     
=========================================
  Files         657      660      +3     
  Lines       18515    19555   +1040     
=========================================
+ Hits         9482    10465    +983     
- Misses       9033     9090     +57
Impacted Files Coverage Δ
babel.config.js 0% <0%> (ø)
discovery-client/src/cli.js 0% <0%> (ø) ⬆️
api/src/opentrons/api/models.py 84.61% <0%> (-6.57%) ⬇️
api/src/opentrons/main.py 27.9% <0%> (-0.67%) ⬇️
api/src/opentrons/hardware_control/controller.py 63.7% <0%> (-0.23%) ⬇️
api/src/opentrons/hardware_control/pipette.py 98.34% <0%> (-0.21%) ⬇️
api/src/opentrons/protocol_api/labware.py 94.03% <0%> (-0.15%) ⬇️
...onents/StepEditForm/FlowRateField/FlowRateField.js 0% <0%> (ø) ⬆️
...l-designer/src/top-selectors/tip-contents/index.js 0% <0%> (ø) ⬆️
app/src/index.js 0% <0%> (ø) ⬆️
... and 69 more

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 cca529c...00b6a96. Read the comment docs.

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.

🎏

  • app
  • app-shell
  • components
  • discovery-client

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.

  • labware-designer
  • protocol-designer
  • protocol-library-kludge
  • shared-data

Make sure to fix typo in webpack-config/lib/env.js before merge but this looks great! I like the analyzer.

ENABLE_ANALYZER:
process.env.ANALYZER &&
process.env.ANALYZER !== 'false' &&
process.env.ANALZER !== '0',
Copy link
Contributor

@IanLondon IanLondon Jan 4, 2019

Choose a reason for hiding this comment

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

ANALZER = typo

also not sure why it's !== 'false instead of (ANALYZER === 'true' || ANALYZER === '1')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good catch! Will fix up

The !== stuff is based on my own personal expectations. Not at all married to it, but it's an attempt to allow $ANALYZER to be any truthy value to enable the analyzer (e.g. ANALYZER=yes, ANALYZER=foo, ANALYZER=99)

@mcous mcous force-pushed the chore_upgrade-babel branch from 742c6f5 to 00b6a96 Compare January 7, 2019 17:49
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 14dd10b into edge Jan 7, 2019
@mcous mcous deleted the chore_upgrade-babel branch January 7, 2019 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants