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

migration to electron-forge, secure app #277

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bcdrme
Copy link

@bcdrme bcdrme commented Oct 20, 2024

Refactoring

Initial goal

Update and secure the new lobby, be idiomatic to the electron way

  • One renderer process for rendering (aka the VueJS app),
  • one main process to handle "backend stuff" (aka interacting with the host filesystem, calling external services etc...)

It meant removing all node integration stuff, re-enabling all security features including sandbox mode and context isolation

--> refactoring is needed to move node api calls to the main process
--> ended up having to cleanup some logic when rendering concerns and "backend" concerns were intertwined

I quickly found out that it wouldn't be possible without a big update of deps, of electron, of node.

Some dependencies were problematic :

  • sqlite3 -> native lib, very difficult to package into the final executable (I couldn't do it)
  • jazcashutils -> transitive dependencies leaking into our project, can't import this lib in the renderer because of node api dep
  • map-parser -> transitive deps
  • sdfz-demo-parser -> transitiv deps

Changes

Disclaimer, some things are going to be a bit controversial.

Moving all code depending on node apis to main.

Adding preload.ts to conform to the context isolation security feature:

  • preload.ts is the step to bind ipcRenderer to ipcMain
  • *.service.ts, it's the entrypoint of the main process' api

Removed api.ts, replaced by ipc apis, or composables (case of the router), or self contained module (see audio)

Removed sqlite3 and cache-db.ts, replaced by IndexedDB (with dexiejs) in the renderer process.

sqlite3, I really tried to make it work but I hit a wall when it came to generating a working executable.
The concerns were to have a place where we can easily look through what's installed, it's particularly usefull for maps because we can query against our db.
Theses concerns can all be adressed in IndexedDB (the db that ships with chrome).

Removing internal deps (jazcashutils, map-parser, sdfz-demo-parser) by including the code directly in this project.

Overall, reduced the amount of dependencies. Big update, some things were not used anymore, or deprecated, or sometimes we had 2 dep for doing the same thing (see markdown).

Rewrites

rewrite of main/main window to be more idiomatic with electron's way of doing things
rewrite of abstract battle
rewrite of the initialization step of content apis to scan the filesystem for packages, maps, replays. Using chokidar to detect automatically new replays in the replay folder

introduction of the store pattern in Vue so that different components can react to changes.

@p2004a p2004a self-requested a review October 20, 2024 14:12
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.

1 participant