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

Separate electron imports between main and renderer, use IPC to replace most renderer-side electron imports #2720

Merged
merged 24 commits into from
Mar 31, 2024

Conversation

joeyballentine
Copy link
Member

electron provides (and recommends using) these segregated imports (electron/renderer, electron/main, electron/common) in order to ensure you aren't accidentally importing something in the wrong place.

I discovered a few places where we were importing main-process electron code in the renderer and have fixed it, along with modifying (almost) all the electron imports to be the helper ones. (I left one import the same because if I didn't then it thinks electron doesn't exist. Try it yourself, its really strange)

I'm opening this as a draft, because there's still somewhere that is importing main process code where it shouldn't, and it is causing chaiNNer to crash on startup.

@joeyballentine joeyballentine changed the title Separate electron imports between main and renderer Separate electron imports between main and renderer, use IPC to replace most renderer-side electron imports Mar 30, 2024
@joeyballentine
Copy link
Member Author

So, I learned a couple things:

  1. the electron webpack template is bad and you apparently need to externalize the modules for them to be imported properly. This took the most time to debug
  2. the namespace imports do prevent you from using them in the renderer, since you're supposed to only expose stuff via the preload script. This is unlike the main electron import that doesn't care. This means that i had to leave a few as regular electron imports in the meantime, until we swap over to exposing IPC via the preload script
  3. Because of the above, i merged a separate branch into this one where i had replaced a lot of renderer-side imports with IPC calls. Theoretically these could be replaced by instead exposing the clipboard api through the preload script, but for now it was just easier to send that stuff through IPC and have IPC be the only thing still being imported the incorrect way. Then, we can move IPC over to the correct way, then replace some of the IPC with non-IPC in the future.

@joeyballentine joeyballentine marked this pull request as ready for review March 30, 2024 19:27
@joeyballentine
Copy link
Member Author

Ok, back to a draft. i broke chain copy and paste from this

@joeyballentine joeyballentine marked this pull request as draft March 30, 2024 22:14
@joeyballentine joeyballentine marked this pull request as ready for review March 30, 2024 22:17
@joeyballentine
Copy link
Member Author

I think i've tested all the different ways to copy and paste, and everything looks good now.

@joeyballentine joeyballentine merged commit 140509f into main Mar 31, 2024
7 checks passed
@joeyballentine joeyballentine deleted the electron-imports branch March 31, 2024 11:25
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