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

Desktop: Upgrade to Electron 29 #10110

Merged

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Mar 12, 2024

Summary

Upgrades to Electron 29. This will allow re-opening #9784 and moves us to one of the stable versions of Electron.

Notes

  • Requires a Playwright upgrade for tests to pass.
  • Requires force-upgrading chokidar v2 to v3. This is done by adding a resolution for chokidar@^2.0.0 to resolve to v3.5.3 to fix build on MacOS.
    • A dependency of chokidar v2 (an old version of fsevents) was breaking the build.
    • We depend on chokidar v2 because we depend on gulp. Gulp depends on glob-watcher v5, which depends on chokidar v2. There exists a glob-watcher v6 that depends on chokidar v3, but force-upgrading it seems to be more likely to introduce breaking changes.
    • According to its changelog, chokidar's v3 version bump was breaking because it removed support for NodeJS < v8. We use node 18 on deskop, so it should be safe to upgrade.
  • In version 27, Electron removed MacOS 10.13 and 10.14 support.
Notes: Issues present before this upgrade

Issues present before this upgrade (v3.0.0):

  • Occasionally, I've noticed notes disappearing when I try to edit them. Switching away from the folder and re-opening it seems to fix the issue.
  • I'm unable to open new webclipped notes unless I first change to a different folder and back to the original. (Clicking their entry in the note list does nothing).
  • Unable to open resource links.

I plan to open issues for the above in the near future. I'm mentioning them here to record that they are not caused by this upgrade.

Testing

Important

Before testing, merge #10104 and rebuild. Otherwise, resource links will fail to open.

The below tests focus on system theme detection and printing, both of which are mentioned in the list of breaking changes.

  1. Verify that it's possible to build Joplin with yarn dist --publish=never.
  2. Create a new note and attach a video/audio file.
  3. Verify that the video can be played in the note viewer.
  4. Verify that clicking on the video's resource link opens the video in an external application.
  5. Attach a text file to the note.
  6. Open the text file in an editor, make a change, and save.
  7. Switch notes.
  8. Switch back to the original note.
  9. Open the text file and verify that edits to the text file were saved.
  10. Verify that it's possible to open a note in an external editor and changes are saved.
    • MacOS: Used XCode as an external editor.
  11. Verify that :exportPdf works in the command palette.
  12. Attach the exported PDF to the note and verify that the PDF viewer loads.
  13. Verify that the print button on the PDF viewer opens a print dialog.
  14. Verify that it's possible to connect and sync to Joplin server.
    • Note: MacOS: I had an item larger than 250 MB, which caused a "Some items could not be synchronized" warning to appear.
  15. Enable automatic theme switching in settings.
  16. Verify that changing the system theme also changes Joplin's theme.
  17. Verify that help > open profile directory works
  18. Verify that cmd-p opens the command palette and that it's possible to search for a word.
  19. Add an external URL link to https://example.com and verify that clicking on it in the viewer opens a browser.
  20. Verify that file > print can be used to print to PDF.

This has been tested successfully on:

  • MacOS Sonoma 14.4 (most tests in a development version of Joplin)
  • Windows 11 (only tested steps 1-13, ran Playwright tests)
  • Ubuntu 23.10

@@ -69,7 +69,7 @@ const useContextMenu = (props: ContextMenuProps) => {
return intersectingElement && isAncestorOfCodeMirrorEditor(intersectingElement);
}

async function onContextMenu(event: ContextMenuEvent, params: ContextMenuParams) {
async function onContextMenu(event: Event, params: ContextMenuParams) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fixes a compilation error related to a type change in Electron.

@laurent22 laurent22 merged commit 3e34f15 into laurent22:dev Mar 14, 2024
10 checks passed
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.

2 participants