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

Migrate to Electron Forge #4

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
Draft

Migrate to Electron Forge #4

wants to merge 10 commits into from

Conversation

axelrindle
Copy link
Collaborator

@axelrindle axelrindle commented Jun 28, 2023

This PR will migrate the codebase and build process to use Electron Forge + Vite.

Todos

  • Replace import aliases with relative imports
  • Replace electron-builder and custom build scripts with electron-forge
  • The dependency fontmanager-redux seems to be broken or unusable
  • Migrate tests
  • Apply old electron-builder.yml config to Electron Forge
  • Update Pipeline
  • Update Documentation

I will probably add more Todos as I make migration progress.

@axelrindle axelrindle added the enhancement ✨ New feature or request label Jun 28, 2023
@axelrindle axelrindle requested a review from jacobwhall June 28, 2023 12:57
@axelrindle axelrindle self-assigned this Jun 28, 2023
@axelrindle axelrindle marked this pull request as draft June 28, 2023 12:57
@axelrindle axelrindle changed the title WIP: Migrate to Electron Forge Migrate to Electron Forge Jun 28, 2023
Copy link
Owner

@jacobwhall jacobwhall left a comment

Choose a reason for hiding this comment

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

This looks great to me so far! The Electron Forge / Vite setup seems pretty clean, and it's exciting to see it nearly build this early into the migration process. These tools are new to me, I'll read more of their documentation this week so I can offer more valuable feedback.

Running npx electron-forge make made it pretty far, and then exited with the following error during the "Building vite bundles" step:

An unhandled rejection has occurred inside Forge:
RollupError: "tmpdir" is not exported by "__vite-browser-external", imported by "src/renderer/util/fileSystem.js".
at error (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:2245:30)
    at Module.error (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:13572:16)
    at Module.traceVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:13997:29)
    at ModuleScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:12515:39)
    at ReturnValueScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:7073:38)
    at ChildScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:7073:38)
    at ReturnValueScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:7073:38)
    at ChildScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:7073:38)
    at TrackingScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:7073:38)
    at BlockScope.findVariable (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:7073:38)
    at Identifier.bind (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:8235:40)
    at CallExpression.bind (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:5841:23)
    at CallExpression.bind (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:9801:15)
    at CallExpression.bind (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:5837:28)
    at CallExpression.bind (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:9801:15)
    at AssignmentExpression.bind (file:///home/jacob/Documents/marktext/node_modules/rollup/dist/es/shared/node-entry.js:5841:23)

Do you also get this error? I found vitejs/vite#8799 which might help address the issue. It seems that a node polyfill plugin for vite might work, though perhaps there is a more elegant solution.

Excellent work Axel!

@axelrindle
Copy link
Collaborator Author

axelrindle commented Jun 28, 2023

I think this error is caused by the Renderer Process trying to access Node.js APIs. While this is generally possible by enabling the nodeIntegration for a BrowserWindow, Vite might be having problems with that.

@jacobwhall jacobwhall mentioned this pull request Jun 28, 2023
@axelrindle axelrindle linked an issue Jun 28, 2023 that may be closed by this pull request
@axelrindle axelrindle added this to the v0.18.0 milestone Jun 28, 2023
@axelrindle axelrindle linked an issue Jun 30, 2023 that may be closed by this pull request
@axelrindle
Copy link
Collaborator Author

Just a small update: I managed to get the application to start with the new Vite workflow, but the renderer does not work yet.

image

@jzhang-gd
Copy link

My work platform is Mac and I’m new to using the MarkText, are there any updates? I'm so excited to find this repo and very appreciative of all of you do the excellent work to maintain it.

@jacobwhall
Copy link
Owner

Hey @jzhang-gd, thanks for your interest in our work. Unfortunately I only have the bandwidth to build updated versions of this MarkText fork for Linux. I welcome contributions (like this PR!) to update the build process for MacOS, Windows, or other operating systems. As a community of volunteers, we rely on each other's time and expertise to improve this project.

If you would like to receive updates on MacOS support specifically, I recommend subscribing for notifications on issue #2. There is a button to do this on the right side of the page on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Target MacOS, Windows with release builds Update mermaid
3 participants