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

WIP Attempt to Migrate to Electron Forge + GitHub Actions for CI/CD #255

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

seanoliver
Copy link
Collaborator

This commit attempts to migrate our deployment process from a manual one to an automated CI/CD process using Electron Forge (https://www.electronforge.io/) to manage signing, notarizing, and building the app. Doing this will provide these benefits:

Here's the primary steps I took thus far (though I branched off in a few different directions for various explorations I can walk through if interested):

  1. Cloned the Webpack + Typescript sample app (https://www.electronforge.io/templates/typescript-+-webpack-template) to see the files needed to build a project with Forge.
  2. Compared the configuration in our app with the sample app and migrated over the key details. The big things I did here are:
    1. Created forge.config.js which contains the configuration for forge. Right now, all of the stuff related to signing and notarizing is commented out, as are some of the icon image paths (was mainly trying to get the thing to build first).
      1. Also note that this file currently only covers the various platform makers and github publication. It references our existing webpack configuration files, but it doesn't implement any of the webpack configuration within Forge — this is possible, but problematic because TypeScript.
      2. I have a separate fork locally that I can share which has all of the webpack configuration directly embedded in the forge config if you're interested in checking that out.
    2. Updated location of main in package.json to point to our webpack configuration at webpack.config.main.prod.ts and removed the manual build configurations. Note the original package.json file is preserved in package-old.json for easier comparison.
  3. Added .github/workflows/config.yml which installs build dependencies and manages the signing and notarizing as part of CI/CD. There are some keys needed here as well.
  4. Added add-osx-cert.sh , a shell script that sets up a temporary keychain on a macOS machine, imports a certificate into it, and configures it for code signing as part of CI/CD. The github action mentioned above calls this.
  5. Temporarily commented out the environment check script because for some reason it wasn't properly distinguishing between production and development. I traced the issue to the call in webpack.config.renderer.prod.ts (line 19) and the decision to run the prod vs. dev webpack configuration which is made on lines 18 and 123 in forge.config.ts.

We're 90% of the way there, but I keep coming back to the issue that it doesn't like that we're using ESM syntax for imports and exports in main.ts even though we can use it just fine in all of our other files. This seems like an easy thing to fix, but each strategy I try creates a new problem with import / export compatibility because we're ultimately using a mix of ESM and require syntax in certain files.

Happy to go deep on any of this as I'd love to see this thing make it across the finish line! Unfortunately I think I've taken it about as far as I can without further help!

@swyxio
Copy link
Contributor

swyxio commented Sep 29, 2023

. This seems like an easy thing to fix, but each strategy I try creates a new problem with import / export compatibility because we're ultimately using a mix of ESM and require syntax in certain files.

this is the entire bull case for bun

Comment on lines +32 to +35
// This allows TypeScript to pick up the magic constants that's auto-generated by Forge's Webpack
// plugin that tells the Electron app where to look for the Webpack-bundled app code (depending on
// whether you're running in development or production).
declare const MAIN_WINDOW_WEBPACK_ENTRY: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

appreciate the comments

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