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

Upgrade to Electron 15 #812

Conversation

NotWearingPants
Copy link
Contributor

@NotWearingPants NotWearingPants commented Oct 3, 2021

Fixes #807 and also brings ueli back to supported Electron versions (the latest!).

I've gone over all of Electron's breaking changes up to version 15, and the only ones affecting ueli are contextIsolation which defaults to true since v12, and the electron.remote module which is deprecated since v12 and removed in v14.

What I did:

  • Update electron to ^15.1.0, the latest version
  • Added an explicit contextIsolation: false to keep the current behavior for now
  • ueli specified enableRemoteModule: true until now because electron-store uses electron.remote.
    But the remote module was removed in v14, so I've updated electron-store to ^8.0.1 (latest) which now uses IPC instead of remote.
  • electron-store.Store is generic on the type of the settings, but electron-stores update changed the default from any to unknown, so I had to specify explicit types for the store in both config & favorites.
  • Finally, updating electron-store also updated it's dependency type-fest.
    type-fest had an import of typescript's esnext definitions, so ueli's usage of String.replaceAll compiled, but type-fest removed this import (Fix TS type reference sindresorhus/type-fest#187) which is good, and so now we must specify we depend on esnext.string in tsconfig.json for replaceAll.

@oliverschwendener
Copy link
Owner

Thanks for your efforts! I quickly tested it on my Macbook and it works but the startup time is way slower. It takes around 10s for the app to respond when triggering the global shortcut to show the window. Any idea what that could be?

@NotWearingPants
Copy link
Contributor Author

I've tested both the dev branch and my branch (and made sure to rm -rf node_modules && yarn), and they both take 3-4 seconds on my machine (a small laptop with below average performance) when measuring from the first line of main.ts until the first time the IPC mainWindowHasBeenShown reaches the main window while I'm mashing the global shortcut nonstop.

So I have no idea why your Macbook takes 10 seconds, can you try to switch back and forth and see if it really is this branch? Maybe your macbook had a bad day, or maybe there's a mac-only issue.

@IRlyLikeBac0n
Copy link

Hi, any update on this? I really valued the dictionary feature, so wish to see #807 resolved. Seems this is the easiest/best solution to that, with very little potential downside.

@oliverschwendener
Copy link
Owner

No success so far. I disabled all plugins to see if one of the plugins is causing the issue but it's still the same. I measured the time it takes to create the main window and it's arond 15s which is a big downside in my opinion ;) I don't want to merge this PR as long as the long startup time is an issue. Unfortunately currently I have very little time to look into this issue, so let's hope that someone might find a clue.

@EdRice4
Copy link

EdRice4 commented Jan 22, 2022

I want to introduce myself by first thanking you, @oliverschwendener, as well as everyone else who has contributed to ueli development; it has become an essential application for me! Secondly, that I am a novice programmer especially with respect to node.js development. However, because I use and enjoy ueli so much and miss the dictionary functionality, I want to contribute in anyway I can.

That being, said, and while this is not much, I want to say that after checking out this PR and testing on my machine, I had no issues with startup time delays: Using the global shortcut, the main window appears just as quickly as it does with the stable version and #807 is resolved. I am running it on Windows 11 so perhaps the issue is MacOS specific as @NotWearingPants suggested?

@oliverschwendener
Copy link
Owner

Somehow I managed to upgrade electron to version 17 without having the startup lag issue on macOS. So i consider this issue as resolved and I will close this PR. Thanks alot again for your efforts and the contribution and sorry that we were not able to merge your work but it's highly appreciated!

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.

Dictionary plugin started failing due to certificate issues
5 participants