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

chore(app-shell): Upgrade the Electron version to 21 #11537

Merged
merged 99 commits into from
Oct 26, 2022

Conversation

koji
Copy link
Contributor

@koji koji commented Oct 5, 2022

Overview

This upgrade is for the ODD app. With Electron version 21, the ODD app works as expected (as well as the current app version). However, there are a couple of issues with this upgrade.

  1. nan AccessorSignatures
    chore: overload deprecated AccessorSignatures nodejs/nan#943

-> The current solution is to use patch-package. Once nan issue is solved, we will remove this.
nan maintainer merged the PR for this issue and the packages use nan will be updated the nan version.
Once updates are done, we can remove this patch.

  1. v8 compile
    https://chromium.googlesource.com/v8/v8/+/8fd20298e5e9fdb69614ba37be1cba8930049dd9
    node v16.8.0 can run make -C app-shell dist-ot3 without any errors.
    later versions need to run CPPFLAGS="-std=c++17" make -C app-shell dist-ot3

There are a couple of tasks for this update in the near future.

  • remove the patch
    Once packages use nan update it to the latest version, we will be able to remove patch

  • update electron-builder (ideally without alpha)
    Currently we need to use "24.0.0-alpha.1" for this issue.

Changelog

  • upgrade Electron version to v21
  • upgrade packages related to the Electron app build process
  • update node-version in workflows from v14 to v16
  • upgrade windows from 2019 to 2022 for app-shell build
  • move electron and related packages from ./package.json to ./app-shell/pakcage.json
  • update DEV_SETUP.md
  • add electron.d.ts to extend File type the previous version of Electron added path to File

Review requests

  • the app works as expected

Risk assessment

medium
This PR will need Q&A team support since this change is related to the entire desktop app.

@koji koji requested review from shlokamin, mcous, sfoster1 and brenthagen and removed request for mcous, sfoster1, shlokamin and brenthagen October 6, 2022 16:21
@koji koji added chore WIP blocked Ticket or PR is blocked by other work labels Oct 6, 2022
@koji
Copy link
Contributor Author

koji commented Oct 6, 2022

  • update a few packages (minor updates only)
    - [ ] update packages for app-shell once nan issue is solved --> patch-package

@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #11537 (5ca6fc9) into edge (493253a) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #11537      +/-   ##
==========================================
- Coverage   75.00%   75.00%   -0.01%     
==========================================
  Files        2057     2059       +2     
  Lines       57435    57446      +11     
  Branches     5985     5988       +3     
==========================================
+ Hits        43079    43087       +8     
- Misses      12949    12950       +1     
- Partials     1407     1409       +2     
Flag Coverage Δ
app 75.26% <ø> (-0.01%) ⬇️
components 65.01% <ø> (ø)
labware-library 49.74% <ø> (ø)
protocol-designer 45.86% <ø> (ø)
react-api-client 83.05% <ø> (ø)
shared-data 84.97% <ø> (ø)
step-generation 88.46% <ø> (ø)
update-server 69.71% <ø> (ø)
usb-bridge 94.40% <ø> (ø)

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

Impacted Files Coverage Δ
.../src/molecules/WizardRequiredLabwareList/index.tsx
...rc/molecules/WizardRequiredEquipmentList/index.tsx 75.00% <0.00%> (ø)
app/src/molecules/GenericWizardTile/index.tsx 83.33% <0.00%> (ø)
...les/WizardRequiredEquipmentList/equipmentImages.ts 100.00% <0.00%> (ø)

@y3rsh
Copy link
Member

y3rsh commented Oct 25, 2022

The .exe and .dmg work, connect to robots by USB and WiFi, and import custom labware.

@koji
Copy link
Contributor Author

koji commented Oct 25, 2022

The .exe and .dmg work, connect to robots by USB and WiFi, and import custom labware.

Oh, thank you for checking!

koji and others added 2 commits October 25, 2022 11:22
Merge branch 'edge' into chore_update-electron-to-21
@koji
Copy link
Contributor Author

koji commented Oct 25, 2022

@koji koji removed the DO NOT MERGE Indicates a PR should not be merged, even if there's a shiny green merge button available label Oct 25, 2022
@koji koji merged commit d1a9618 into edge Oct 26, 2022
@koji koji deleted the chore_update-electron-to-21 branch November 6, 2022 07:41
@koji koji restored the chore_update-electron-to-21 branch March 13, 2023 17:17
@shlokamin shlokamin deleted the chore_update-electron-to-21 branch March 15, 2023 14:44
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.

6 participants