build(js): upgrade babel and webpack toolchains #4959

mcous · 2020-02-10T19:55:58Z

overview

This PR upgrades the following portions of our JS build:

Babel
- Now with nullish coalescing!
- Now with support for explicit inexact Flow objects ({ ... })
Webpack
- Felt like a thing to do
- Webpack v5 is currently in beta, so want to make sure we're up to date on v4 to minimize future upgrade pain
React Hot Loader
- Now with hooks support
Jest
- Because babel was upgraded

review requests

Smoke tests require for all front end apps:

Protocol designer
App
Labware Library
Protocol Library Kludge (deckmap)

labware-library/package.json

mcous · 2020-02-10T19:58:31Z

labware-library/package.json

    "react-router-dom": "^5.0.1",
    "yup": "^0.27.0"
-  },
-  "devDependencies": {
-    "react-snap": "^1.23.0"


All devDependencies go in monorepo package.json, where this entry already exists

mcous · 2020-02-10T19:59:07Z

package.json

    "ajv": "^6.10.2",
    "aws-sdk": "^2.493.0",
-    "babel-eslint": "^10.0.3",
-    "babel-jest": "^24.9.0",
+    "babel-eslint": "^11.0.0-beta.2",


Beta release required for flow inexact object syntax; keep an eye on this

That's annoying — let's def keep an eye out

package.json

codecov · 2020-02-10T20:21:25Z

Codecov Report

Merging #4959 into edge will decrease coverage by 0.05%.
The diff coverage is 26.38%.

@@            Coverage Diff             @@
##             edge    #4959      +/-   ##
==========================================
- Coverage   68.74%   68.69%   -0.06%     
==========================================
  Files        1076     1076              
  Lines       35871    35916      +45     
==========================================
+ Hits        24660    24672      +12     
- Misses      11211    11244      +33

Impacted Files	Coverage Δ
app/src/robot/reducer/index.js	`100% <ø> (ø)`	⬆️
...mponents/AppSettings/UpdateApp/UpdateAppMessage.js	`0% <ø> (ø)`	⬆️
...p/src/components/AppSettings/AddManualIp/IpItem.js	`0% <ø> (ø)`	⬆️
app/src/logger.js	`0% <ø> (ø)`	⬆️
...p/src/components/TipProbe/InstrumentMovingPanel.js	`0% <ø> (ø)`	⬆️
discovery-client/src/mdns-browser.js	`33.33% <ø> (ø)`	⬆️
app/src/components/MenuPanel/index.js	`0% <ø> (ø)`	⬆️
app/src/components/FileInfo/MissingItemWarning.js	`0% <ø> (ø)`	⬆️
app/src/discovery/reducer.js	`100% <ø> (ø)`	⬆️
app/src/components/nav-bar/index.js	`0% <ø> (ø)`	⬆️
... and 179 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 e7fa264...95ae8c2. Read the comment docs.

iansolano

lgtm

shlokamin

Not sure what's going on with the image source tags in PD, but they're being resolved to src="[object Module]" which is causing this:

Any idea why this might be?

shlokamin · 2020-02-20T15:26:02Z

app/package.json

-    "react-dom": "^16.8.6",
-    "react-hot-loader": "^3.0.0-beta.7",
+    "react": "16.8.6",
+    "react-dom": "16.8.6",


Just curious — why is it that we were previously using semantic versioning for react and not we're not?

Judgement call and a safety thing:

In order for hooks support of react-hot-loader, we have to swap out react-dom with @hot-loader/react-dom

The version of @hot-loader/react-dom needs to match react-dom

We're already locking our dependencies implicitly via the lock file, so locking them down explicitly here felt like the safest thing to do

Longer term, I think we should probably lock all of our production dependencies explicitely (I'm generally good keeping dev deps on semver). That being said, I'd rather not mess with all our app deps in this PR

shlokamin · 2020-02-20T15:28:24Z

components/package.json

@@ -17,7 +17,7 @@
  "peerDependencies": {
    "classnames": "^2.2.5",
    "lodash": "^4.17.4",
-    "react": "^16.8.6",
+    "react": "16.8.6",
    "react-router-dom": "^5.0.1"


Same question about semantic versioning here

IanLondon

On top of what Shlok said: animations for Transfer step paths (popover on Path field buttons) also broken, probably other images as well. There's a lot of requires for images in PD...

Hot reloading seems to work the same as before. PD has an issue with React-DnD not playing well with hot reloads so if you're hot reloading on the "DESIGN" tab, you only get one hot reload before it crashes, that's the same as on edge and obvs out of scope of this PR.

--

Besides PD, I tested protocol-library-kludge -- all good there!

mcous · 2020-02-20T17:34:27Z

Sorry about the image thing, forgot to read the changelog of file-loader after bumping it from v4 to v5. Pushed the short-term fix to the webpack config with the longer-term fix in a TODO comment

shlokamin

LGTM — image tags + animations seem to be working now!

IanLondon

LL, LC, and PD looking good image-wise

build(js): upgrade babel and webpack toolchains

1cf6246

mcous requested review from a team, Kadee80, IanLondon, iansolano and amitlissack and removed request for a team February 10, 2020 19:56

mcous added chore WIP JS Tech Debt DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available labels Feb 10, 2020

mcous commented Feb 10, 2020

View reviewed changes

labware-library/package.json Outdated Show resolved Hide resolved

mcous commented Feb 10, 2020

View reviewed changes

package.json Show resolved Hide resolved

mcous removed the DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available label Feb 11, 2020

fixup: bump some packages and lock react

b81a6d1

mcous added ready for review and removed WIP labels Feb 18, 2020

mcous marked this pull request as ready for review February 18, 2020 18:52

mcous requested review from a team as code owners February 18, 2020 18:52

mcous added this to the CPX Sprint 3 milestone Feb 18, 2020

mcous added DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available and removed DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available ready for review labels Feb 19, 2020

iansolano approved these changes Feb 19, 2020

View reviewed changes

Merge branch 'edge' into chore_upgrade-js-tools

9a9246a

mcous added the ready for review label Feb 19, 2020

shlokamin requested changes Feb 20, 2020

View reviewed changes

IanLondon suggested changes Feb 20, 2020

View reviewed changes

fixup: fix file-loader config after upgrade

e7fa264

mcous requested review from IanLondon and shlokamin February 20, 2020 17:33

shlokamin approved these changes Feb 21, 2020

View reviewed changes

IanLondon approved these changes Feb 21, 2020

View reviewed changes

Merge branch 'edge' into chore_upgrade-js-tools

95ae8c2

mcous merged commit ec0645a into edge Feb 21, 2020

mcous deleted the chore_upgrade-js-tools branch February 21, 2020 18:11

mcous mentioned this pull request Apr 29, 2021

chore(components): sub in storybook for styleguidist #7549

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build(js): upgrade babel and webpack toolchains #4959

build(js): upgrade babel and webpack toolchains #4959

mcous commented Feb 10, 2020 •

edited

Loading

mcous Feb 10, 2020

mcous Feb 10, 2020

shlokamin Feb 20, 2020

codecov bot commented Feb 10, 2020 •

edited

Loading

iansolano left a comment

shlokamin left a comment

shlokamin Feb 20, 2020

mcous Feb 20, 2020

shlokamin Feb 20, 2020

IanLondon left a comment

mcous commented Feb 20, 2020

shlokamin left a comment

IanLondon left a comment

build(js): upgrade babel and webpack toolchains #4959

build(js): upgrade babel and webpack toolchains #4959

Conversation

mcous commented Feb 10, 2020 • edited Loading

overview

review requests

mcous Feb 10, 2020

Choose a reason for hiding this comment

mcous Feb 10, 2020

Choose a reason for hiding this comment

shlokamin Feb 20, 2020

Choose a reason for hiding this comment

codecov bot commented Feb 10, 2020 • edited Loading

Codecov Report

iansolano left a comment

Choose a reason for hiding this comment

shlokamin left a comment

Choose a reason for hiding this comment

shlokamin Feb 20, 2020

Choose a reason for hiding this comment

mcous Feb 20, 2020

Choose a reason for hiding this comment

shlokamin Feb 20, 2020

Choose a reason for hiding this comment

IanLondon left a comment

Choose a reason for hiding this comment

mcous commented Feb 20, 2020

shlokamin left a comment

Choose a reason for hiding this comment

IanLondon left a comment

Choose a reason for hiding this comment

mcous commented Feb 10, 2020 •

edited

Loading

codecov bot commented Feb 10, 2020 •

edited

Loading