-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
feat: electron desktop app without express server #1136
base: main
Are you sure you want to change the base?
Conversation
hey this looks awesome. but not sure if this will require more maintenance or less looks like there are quite a lot of stuff you are ding to handle http request and passing to node adaptor |
yeah, it might need more tweaks for handling http because, in this setup, you’re not actually sending requests to an http server but instead to the main thread. that’s why i added cookie forwarding to persist the api key. i intercept the request and set it in a cookie.
the benefit of this approach is that you can directly use electron apis in remix server loaders and actions. this opens up a lot of potential for adding features to the desktop app. i call it easy to maintain because it makes adopting upstream changes much simpler. it keeps the web app separate from the electron app. the electron app essentially just utilizes the remix server bundle without requiring any additional changes. as you see, no changes to the app folder. |
yeah I love it. its exactly what I thought you are doing. I think this is better approach as its not opening a new port in the os.
yeah I agree, even the express approach is not changing anything thats specific to electron. that PR is just de-cloudflaring it cuz we want to add ability to host other platforms like netlify and vercel. by "more maintanance" what i meant was, the index.ts is too big and not very organized. I would suggest if you can refactor this index.ts code into more readable smaller modules so that will be easy to maintain. also you have forwarded the cookies for apikeys. why its a separately implemented effort? the best solution would be not having any specific code for that (just pass what ever cookies are received). for example: it does not have other cookies that were used like providerSettings. |
Oh, yup. in that case, it's totally make sense to replace
Sure. that's easy. I will update it.
Good question. I think I was trying to persist the apiKeys when I added that logic. but I was not sure if I shall persist other cookies. maybe I just store all cookies so all settings will remain the same. will update |
Hi @thecodacus, |
is it ready.. I still see it in draft state |
ah, i didn’t know you were waiting for that. right now, i’m able to run the unpacked version ( i haven’t tested the packaged version yet, and that’s what i’ll do next. i can test it on mac and windows. once i confirm the packaged version works on mac and windows, i’ll mark this pr as ready for review. i’ve been less busy lately, so i should be able to wrap it up soon. |
@thecodacus |
merge conflicts resolved |
@Derek-X-Wang, I am getting this error on mac build |
this is due to the server build not loading correctly, usually caused by path resolution issues. |
dmg usually the dist builds are what we should be publishing as release. as that comes with an installer |
I installed the dmg I built from my side. Cannot reproduce this issue. |
Motivation
Turning bolt.diy into a desktop application will make it easier for non-technical users to access. It also opens up the potential for accessing system-level APIs.
I have seen the awesome PR by @thecodacus, and I still want to share my approach with the bolt.diy community because it takes a different path:
electron
folder.Scope
Limitation
Related
For reference to the existing Electron approach, see this PR.