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

Allow GCS to use latest version of electron #106

Merged
merged 1 commit into from Mar 5, 2020
Merged

Conversation

Luis729
Copy link
Member

@Luis729 Luis729 commented Mar 4, 2020

Why is the change being made?

This change is made because GCS was running on electron v3, a version
made back two years ago. This version is long deprecated and GCS needed
to upgrade to v8 sooner or later. However, electron v5 would not build
at all (we dont consider v4 as it does not support geolocation).

What has changed to address the problem?

This change finds the issue to be the fact electron v5+ turns off the
nodeIntegration field for windows (used to be on by default). This was
breaking our application.

This change sets nodeIntegration to true for all windows created, and
updates electron to v8. Other related fields that were modified from v3
to v8 are updated in this change.

Other minor changes are the following:

How was this change tested?

This change was tested with npm test, running the application and
seeing if it was fine, and running the application from npm run build
and seeing if it was fine (checking console, build status, and if
serialport runs).

Related documents, URLs, commits

https://github.com/electron/electron/releases/tag/v5.0.0

@Luis729 Luis729 added the enhancement New feature or request label Mar 4, 2020
@Luis729 Luis729 requested a review from a team March 4, 2020 11:32
@coveralls
Copy link

Pull Request Test Coverage Report for Build 611

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 55.901%

Totals Coverage Status
Change from base Build 193: 0.0%
Covered Lines: 62
Relevant Lines: 101

💛 - Coveralls

@NGCP NGCP deleted a comment from coveralls Mar 4, 2020
 ## Why is the change being made?

This change is made because GCS was running on electron v3, a version
made back two years ago. This version is long deprecated and GCS needed
to upgrade to v8 sooner or later. However, electron v5 would not build
at all (we dont consider v4 as it does not support geolocation).

 ## What has changed to address the problem?

This change finds the issue to be the fact electron v5+ turns off the
`nodeIntegration` field for windows (used to be on by default). This was
breaking our application.

This change sets `nodeIntegration` to true for all windows created, and
updates electron to v8. Other related fields that were modified from v3
to v8 are updated in this change.

Other minor changes are the following:
- removing dependency on `electron-rebuild` and setting `preinstall` script
to `electron-builder install-app-deps` in `package.json`
- removing `build` field in `package.json` as `electron-rebuild` is no
longer a dependency
- added dependency to `@types/node` v12 as default version v13 breaks tests with
electron v8 (see electron/electron#21612)

 ## How was this change tested?

This change was tested with `npm test`, running the application and
seeing if it was fine, and running the application from `npm run build`
and seeing if it was fine (checking console, build status, and if
serialport runs).

 ## Related documents, URLs, commits

https://github.com/electron/electron/releases/tag/v5.0.0
@Luis729 Luis729 merged commit 9a2bfdb into master Mar 5, 2020
@Luis729 Luis729 deleted the electron@latest branch March 5, 2020 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants