itwinjs-core icon indicating copy to clipboard operation
itwinjs-core copied to clipboard

Add support for Electron versions from 15 to 17

Open GytisCepk opened this issue 3 years ago • 7 comments

Changes

  • Changed "electron" peer dependency version from ^14.0.0 to >=14.0.0 <18.0.0 in @itwin/core-backend and @itwin/certa packages. This allows users to choose between several major Electron versions.
  • Increased "electron" dev dependency to ^17.0.0 to use latest supported electron version in development.

Considerations in supporting multiple major versions of Electron

Electron breaking changes

Electron breaking changes list. From what I can tell non of the breaking changes on the list break our API or behavior.

Node versions

Electron supported Node versions per Electron version:

Electron version Node version
14.0.0 14.17
15.0.0 16.5
16.0.0 16.9
17.0.0 16.13

Since we support >=14.17 and >=16.13 Node versions, this should be fine (unless Electron requires exact Node version ?).

Testing

Changes were tested with Electron versions 14.2.9, 15.5.7 and 17.4.11. full-stack-test tests with electron as a dependency pass and appui, display and ui test apps seem to work as intended.

GytisCepk avatar Jul 15 '22 10:07 GytisCepk

This PR looks good to me. Any reason it's still sitting in a draft state?

There is a problem with Certa tool not working properly while debugging on Electron 19. Working on that right now.

GytisCepk avatar Jul 20 '22 12:07 GytisCepk

@calebmshafer, @aruniverse

Support for Electron 18 and 19 is still blocked by AsyncLocalStorage problem. It seems to be a V8 bug (Issue #35043). My recommendation is to add support for Electron versions 15, 16 and 17 for now. Once the V8 fix is available, we should revisit Electron support once again.

GytisCepk avatar Aug 04 '22 08:08 GytisCepk

@calebmshafer, @aruniverse

Support for Electron 18 and 19 is still blocked by AsyncLocalStorage problem. It seems to be a V8 bug (Issue #35043). My recommendation is to add support for Electron versions 15, 16 and 17 for now. Once the V8 fix is available, we should revisit Electron support once again.

@GytisCepk That sounds good for now. Can you add a note in the NextVersion.md to explain it?

calebmshafer avatar Aug 04 '22 10:08 calebmshafer

Taking display-test-app for a spin (node 16.16.0, Ubuntu 20.04).

pmconne avatar Aug 04 '22 12:08 pmconne

I am enqueuing an ImageTests run against this branch. Please do not complete this PR until the results are analyzed.

pmconne avatar Aug 04 '22 12:08 pmconne

display-test-app appears to be working normally and ImageTests reports no problems.

pmconne avatar Aug 04 '22 21:08 pmconne

@calebmshafer @wgoehrig this is awaiting your review.

pmconne avatar Aug 08 '22 17:08 pmconne