chore(monorepo): Update Electron to v33.2.1 #17010

mjhuff · 2024-12-02T20:34:54Z

Overview

This PR continues the work started in #15894, cherry-picking several commits and completes updating Electron to 33.2.1. There are a couple dependency updates as well: node gets a big bump to 22.11.0,electron-rebuild bumps to 3.7.1, and electron-builder bumps to 25.1.8. This PR does not bump USB-related dependencies, since USB works as is (and if we're going to bump those dependencies, it would be nice to do them in an isolated PR).

Opentrons/oe-core#180 bumps node for builds.

NOTE: This PR should be merged alongside the oe-core PR.

HUGE thanks to @shlokamin and @koji for all the work they put into this that made my life so much easier!

Functional Changes

`File.path` workarounds

The removal of file.path directly affects protocol and labware uploading in the desktop app. The docs recommend exposing a web util via context bridge, but because enabling context bridge breaks a lot of existing functionality, this PR exposes it on the global app object. See 556f704. The end result is effectively the same. There are many different approaches one could take to this, but the approach in this commit has by far the smallest blast radius while ensuring no functional change (DnD does not play well when trying to do things like having the shell actually show the file upload dialogue).

Electron Rebuilder

Electron rebuilder has an internal dependency on node-abi, which provides some mappings necessary to build electron. The problem is when Electron rebuilder releases lag behind Electron releases, node-abi is out of date, the mappings for node to electron versions don't exist, and the build fails. This is nothing new - it seems to be a constant problem. A quick search for "electron-rebuilder node-abi" shows the problem happens again and again and again, year and after.

The easiest, least invasive way I found was to effectively specify the node mapping manually in the app-shell(odd) package.json script.

CI Ubuntu Server Bump

For a lot of CI, the current ubuntu server version wasn't playing nicely with the updated serialport native module rebuilding, specifically GCC. Bumping the server version solves this.

Test Plan and Hands on Testing

Before testing:

Switch nvs to 22 (the default is 22.11, the LTS version).
Do a make teardown && make setup.

Desktop Smoke Test

Protocol uploading (and DnD functionality) and custom labware uploading is fixed. See this issue for context.
Run a protocol.
Desktop electron-updater flow works.
Pushing an artifact to the ODD works.
USB detection and general app usage is fixed.
Run LPC.
Devtools work (at least as well as they have been working).
Test on Windows

ODD Smoke Test

The uploaded artifact actually runs.
USB uploading an update artifact on the Flex works.
Devtools work (at least as well as they have been working).
Running an uploaded protocol works.

Changelog

Updated Electron to v33.2.1.
Updated Node to 22.11.0
Bumped electron rebuild to 3.7.1
Bumped electron builder to 25.1.8.

Review requests

Are there any other node/electron cases that may break that should be smoke-tested?

Risk assessment

high

koji · 2024-12-02T20:48:53Z

@mjhuff
maybe utilize this? #15894

codecov · 2024-12-03T02:09:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.21%. Comparing base (1c0a01c) to head (2c40681).
Report is 14 commits behind head on edge.

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #17010      +/-   ##
==========================================
+ Coverage   79.20%   79.21%   +0.01%     
==========================================
  Files         120      120              
  Lines        4516     4533      +17     
==========================================
+ Hits         3577     3591      +14     
- Misses        939      942       +3

Flag	Coverage Δ
shared-data	`74.00% <ø> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

mjhuff · 2024-12-04T17:39:37Z

...Desktop/ChooseRobotToRunProtocolSlideout/__tests__/ChooseRobotToRunProtocolSlideout.test.tsx

-    await waitFor(() =>
-      screen.findByText('Add the required CSV file to continue.')
-    )
+    await screen.findByText('Add the required CSV file to continue.')


The previous code was a race condition, making the test flaky.

koji · 2024-12-04T19:13:04Z

DEV_SETUP.md

@@ -202,7 +202,7 @@ Once you are inside the repository for the first time, you should do two things:
 3. Run `python --version` to confirm your chosen version. If you get the incorrect version and you're using an Apple silicon Mac, try running `eval "$(pyenv init --path)"` and then `pyenv local 3.10.13`. Then check `python --version` again.

 ```shell
-# confirm Node v18
+# confirm Node v22


maybe we need to avoid using v22.5.0
I don't remember exactly but when I started updating node version, at that time the latest version was 22.5.0 but it had a few issues, and I ended up using v22.4.

if someone runs nvs add 22 first time, it won't have issue since nvs will install lts version.

probably need to add v22.11+ because of the following

"node": ">=22.11.0"

koji · 2024-12-04T19:14:47Z

app-shell/package.json

@@ -50,7 +50,7 @@
    "electron-localshortcut": "3.2.1",
    "electron-devtools-installer": "3.2.0",
    "electron-store": "5.1.1",
-    "electron-updater": "4.1.2",
+    "electron-updater": "6.3.9",


koji · 2024-12-04T19:16:37Z

app/package.json

@@ -77,6 +77,7 @@
    "@types/node-fetch": "2.6.11",
    "@types/styled-components": "^5.1.26",
    "axios": "^0.21.1",
+    "electron-updater": "6.3.9",


nit
maybe we should move this to root package.json?

koji · 2024-12-04T19:25:56Z

@mjhuff
I strongly recommend checking if you haven’t tested this branch on Windows.

mjhuff · 2024-12-04T19:29:52Z

@mjhuff I strongly recommend checking if you haven’t tested this branch on Windows.

Good callout, thanks. I did test this, but I forgot to add it to the test cases above. Added it to the PR now!

koji

tested Desktop (mac) and it worked as expected.
thank you for updating Electron version and nodejs version.

koji · 2024-12-05T16:18:40Z

fyi
checked dev and build options with pd and ll. they worked as expected with node v22.11.0

The electron update made it no longer possible to access the file path from the renderer process, see #17010 for more details. That PR did not update one important case: uploading zip files. Currently, fun behavior happens when uploading a system zip, such as throwing an error or better yet, pushing an old, cache file instead of the expected system file. This commit fixes that.

koji and others added 3 commits August 5, 2024 10:15

update setup doc

8d18186

update interfaces and expose types in the app

54882b5

remove unused imports

d56728f

mjhuff and others added 2 commits December 2, 2024 17:18

refactor(app): update electron and required dependencies

d0cc3dc

update checkout and setup-node to v4

3716c24

mjhuff force-pushed the update-electron-33.2.1 branch 8 times, most recently from eb01475 to 751ada5 Compare December 3, 2024 01:39

mjhuff added 2 commits December 2, 2024 21:01

refactor(ci): update LL CI ubuntu version to support serialport

631821d

refactor(monorepo): clean up failing ci

13f8c0f

mjhuff force-pushed the update-electron-33.2.1 branch from 751ada5 to 2c40681 Compare December 3, 2024 02:02

mjhuff force-pushed the update-electron-33.2.1 branch from 2c40681 to 13f8c0f Compare December 3, 2024 03:38

refactor(app): use node 22.11.0 over 22.4.0

42aa948

mjhuff mentioned this pull request Dec 3, 2024

chore(recipe): update nodejs to 22.11.0 Opentrons/oe-core#180

Merged

2 tasks

fix(app): fix file uploading on the desktop app

556f704

mjhuff marked this pull request as ready for review December 4, 2024 17:18

mjhuff requested review from a team as code owners December 4, 2024 17:18

mjhuff requested review from TamarZanzouri, koji, sfoster1 and shlokamin and removed request for a team December 4, 2024 17:18

mjhuff requested review from smb2268, ncdiehl11 and jerader December 4, 2024 17:18

mjhuff commented Dec 4, 2024

View reviewed changes

koji reviewed Dec 4, 2024

View reviewed changes

mjhuff added 2 commits December 4, 2024 14:38

specify version in docs

0d10456

electron-updater to root package.json

6898e67

koji self-requested a review December 5, 2024 15:14

koji approved these changes Dec 5, 2024

View reviewed changes

mjhuff merged commit a714e8d into edge Dec 5, 2024
88 checks passed

mjhuff deleted the update-electron-33.2.1 branch December 5, 2024 17:04

This was referenced Dec 6, 2024

fix(app-shell): fix electron-updater failures #17056

Merged

fix(app): fix manual file upload #17098

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(monorepo): Update Electron to v33.2.1 #17010

chore(monorepo): Update Electron to v33.2.1 #17010

mjhuff commented Dec 2, 2024 •

edited

Loading

koji commented Dec 2, 2024

codecov bot commented Dec 3, 2024

mjhuff Dec 4, 2024

koji Dec 4, 2024 •

edited

Loading

koji Dec 4, 2024

koji Dec 4, 2024

koji commented Dec 4, 2024

mjhuff commented Dec 4, 2024 •

edited

Loading

koji left a comment

koji commented Dec 5, 2024

chore(monorepo): Update Electron to v33.2.1 #17010

chore(monorepo): Update Electron to v33.2.1 #17010

Conversation

mjhuff commented Dec 2, 2024 • edited Loading

Overview

Functional Changes

File.path workarounds

Electron Rebuilder

CI Ubuntu Server Bump

Test Plan and Hands on Testing

Desktop Smoke Test

ODD Smoke Test

Changelog

Review requests

Risk assessment

koji commented Dec 2, 2024

codecov bot commented Dec 3, 2024

Codecov Report

mjhuff Dec 4, 2024

Choose a reason for hiding this comment

koji Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

koji Dec 4, 2024

Choose a reason for hiding this comment

koji Dec 4, 2024

Choose a reason for hiding this comment

koji commented Dec 4, 2024

mjhuff commented Dec 4, 2024 • edited Loading

koji left a comment

Choose a reason for hiding this comment

koji commented Dec 5, 2024

mjhuff commented Dec 2, 2024 •

edited

Loading

`File.path` workarounds

koji Dec 4, 2024 •

edited

Loading

mjhuff commented Dec 4, 2024 •

edited

Loading