joplin
joplin copied to clipboard
Desktop: Resolves #8258: Upgrade to Electron 25
Summary
Upgrades Electron to v25.2.0.
This resolves #8258.
Notes
- I created a patch that should/seems to resolve the Windows electron-builder issue that was blocking an upgrade to
electron-builder
v23+. - Indirectly, Joplin depended on an outdated version of
nan
which was incompatible with the new version of Electron. Manually removing the entry for this version ofnan
fromyarn.lock
and re-runningyarn install
seems to have fixed the issue. -
alias python=$(which python3)
instead of thePYTHON_PATH=
workaround seems to now be necessary on MacOS... -
electron-rebuild
has been renamed to@electron/rebuild
.
Testing Summary
Tests were done with commit e08d91897bc463185d4b19957742b2b1000b3771
Windows:
Some Windows tests were only done with commit 5c474012969e2dada7f8dd4fa2c421094e38a239, but many were done with both 5c474012969e2dada7f8dd4fa2c421094e38a239 and e08d91897bc463185d4b19957742b2b1000b3771.
-
[x] Check resource attachment
- [x] PDF
- [x] Viewer shows
- [x] Can open print/save dialogs
- [x] Image
- [x] Text file
- [x] Can be edited
- [x] PDF
-
[x] Check external editing
-
[x] Check export/import
- [x] PDF (export only)
- [x] Markdown
- [x] JEX
-
[x] Check sync
- [x] File system
- [x] DropBox
-
[x] Check print
- [x] works, but no print preview available.
-
[x] Check plugins
- [x] js-draw
- [x] remark slides
- [x] templates
- [x] kanban
- [x] Seems to conflict with one of the other plugins (math mode), though maybe just at first launch
- [x] math mode
- [x] simple backup
- [x] note tabs
-
[x] Portable version launches and can view notes
- Launches, but takes about 10 seconds to open. Is this new?
Note: I'm getting a huge diff on Windows after running the build script. Is this expected? (.gitignore and .eslintignore had also changed and all .js files were no longer ignored).
MacOS
-
[x] Check resource attachment
- [x] PDF
- [x] Viewer shows
- [x] Can open print/save dialogs
- [x] Image
- [x] Text file
- [x] Can be edited
- [x] PDF
-
[x] Check external editing
-
[x] Check export/import
- [x] PDF (export only)
- [x] Markdown
- [x] JEX
-
[x] Check sync
- [x] File system
- [x] DropBox
- [x] OneDrive
-
[x] Check print
- Print: (no printers available on the network) when no printer connected
-
- Dialog shows when a printer is connected, however.
- It works in a plugin (reveal.js slides), however...
-
[x] Check plugins
- [x] js-draw
- [x] remark slides
- [x] templates
- [x] kanban
- Seems to conflict with one of the other plugins (math mode), though maybe just at first launch
- [x] math mode
- [x] simple backup
- [x] note tabs
Linux/Fedora
-
[x] Check resource attachment
- [x] PDF
- [x] Viewer shows
- [x] Can open print/save dialogs
- [x] Image
- [x] Text file
- [x] Can be edited
- [x] PDF
-
[x] Check external editing
-
[x] Check export/import
- [x] Markdown
- [x] JEX
-
[x] Check sync
- [x] File system
- [x] DropBox
-
[ ] Check print
- Broken.
- This doesn't seem to be a regression (I get the same
with my main Linux install (older electron)). See https://github.com/laurent22/joplin/issues/8240
-
[x] Check plugins
- [x] js-draw (tested successfully on a different device from the other plugins — was having trouble on the VM).
- [x] remark slides
- [x] templates
- [x] kanban
- Seems to conflict with one of the other plugins (math mode), though maybe just at first launch
- [x] math mode
- [x] simple backup
- [x] note tabs
Thanks, that looks very good at first sight and all checks have passed. For printing, it's always a struggle to get it working and indeed I don't think those are new bugs.
Note: I'm getting a huge diff on Windows after running the build script. Is this expected? (.gitignore and .eslintignore had also changed and all .js files were no longer ignored).
But not on this pull request? Hard to tell without seeing the diff but probably it shouldn't happen
But not on this pull request? Hard to tell without seeing the diff but probably it shouldn't happen
After resetting changes, I just re-ran yarn install ; cd packages/app-desktop ; yarn dist --publish=never
in both Git Bash and PowerShell and can no longer reproduce the issue.
Here's what I was noticing before I reset my changes:
It looks like the line endings in
.gitignore
changed:and thus files are no longer ignored (originally, the
^M
s listed above were not present at the end of the file).There are also some diffs that seem empty (line ending changes?) in other auto-generated files.
Edit: It happened again (running yarn install ; cd packages/app-desktop ; yarn dist --publish=never
in PowerShell):
Large portions of the diff look like this, however:
Based on this, I don't think that the issue is related to this pull request.
(Sorry for the screenshots — I'm running Windows 11 in a VM that doesn't have clipboard sharing and isn't signed in to GitHub).
Yes it looks like it's something else, maybe something related to the autocrlf or safecrlf gitconfig, so we can ignore this for now. Are there other tests you need to run on the Electron upgrade or is it ready to merge?
Are there other tests you need to run on the Electron upgrade or is it ready to merge?
I'm unresolving the above windows installer patch issue. I'm no longer convinced that the patch works. As per comment, the issue might be related to the length of the path to the running process. I'll test again with a long username and report back. If that works, this should be ready to merge.
Are there other tests you need to run on the Electron upgrade or is it ready to merge?
Even though the installer seems to work on Windows (even with spaces, non-ASCII in usernames see tests above), it's possible that it's still affected by https://github.com/electron-userland/electron-builder/issues/6865.
It looks like the installer issue can occur on Windows 11 (and, based on comments on that issue, when the installer is located on the C:\ drive). Thus, I'm guessing that the issue has been fixed, but am not certain.
If users do experience the issue, we can either
- add an additional patch to
electron-builder
that gives users an "ignore" option if already-running versions of the app can't be closed - make the patch revert the commit that I think caused the issue originally
-
create the Windows installer with an earlier version of
electron-builder
than the MacOS/Linux installers.
Thanks @personalizedrefrigerator, I think we can merge then, and if we get any feedback about this during prerelease we can apply one of your suggestions.