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

Electron and Dependencies update #801

Merged
merged 6 commits into from
Jan 23, 2024
Merged

Electron and Dependencies update #801

merged 6 commits into from
Jan 23, 2024

Conversation

Isaac-GC
Copy link
Collaborator

@Isaac-GC Isaac-GC commented Jan 14, 2024

This is a supplemental PR (meant to be an update and replacement for PR#751).

Updates
electron -> 18.3.7 -> 22.3.27
electron-builder 24.4.0 -> 24.9.1
electron-updater 5.2.1 -> 6.1.7
better-sqlite3 7.5.3 -> 9.2.2

electron-rebuild -> updated to @electron/rebuild w/version 3.4.1

Currently, with updated electron versions, there is an issue where the node-abi version can't be automatically detected w/electron-builder. To get around this issue, the node-abi version was added in the "resolutions" portion of the package.json

@Isaac-GC Isaac-GC changed the title Electron and Electron and Dependencies update Jan 14, 2024
@18alantom
Copy link
Member

Seems like it's not finding pre-built binaries for better-sqlite3 under Node 16.14.0 and Darwin causing all tests to fail:
https://github.com/frappe/books/actions/runs/7521667651/job/20472776937?pr=801#step:5:17

(build.yml runs on macOS since it's the only one that builds for all platforms)

@Isaac-GC
Copy link
Collaborator Author

Updated the nodejs version from 16 to 18 in the workflow files and that seemed to fix it.

@18alantom
Copy link
Member

Checked it locally (ARM macOS 13.5) nothing overtly broken, looks good.

Copy link
Contributor

@mildred mildred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why package-lock.json was never committed

Else, LGTM

@Isaac-GC
Copy link
Collaborator Author

Isaac-GC commented Jan 17, 2024

No, looks like it was removed from the master branch since 2018), but did add in yarn.lock

@mildred
Copy link
Contributor

mildred commented Jan 19, 2024

I was thinking there might be a legitimate reason why yarn.lock was not included

@Isaac-GC
Copy link
Collaborator Author

Since Yarn was implemented as the tool of choice by using the yarn command to install dependencies, I understand the yarn.lock takes the place of package-lock.json. I don't really see a conflict that might happen with having the yarn.lock file (though having both of types of lockfiles present in the repo might cause issues)

@18alantom Is/Was there issues previously with having the yarn.lock file present in the repo and/or should there be a package-lock.json as well?

@Isaac-GC Isaac-GC merged commit 2ced2e5 into master Jan 23, 2024
4 checks passed
@Isaac-GC Isaac-GC deleted the pr-751-electron-update branch January 23, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants