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

chore: replace Parcel with Vite #1453

Open
eglitise opened this issue May 9, 2024 · 9 comments
Open

chore: replace Parcel with Vite #1453

eglitise opened this issue May 9, 2024 · 9 comments
Labels
chore Internal changes not visible to the user help wanted Extra attention is needed

Comments

@eglitise
Copy link
Collaborator

eglitise commented May 9, 2024

Parcel v1 is greatly outdated and is holding back many important dependency upgrades, like Electron and antd.
Several possible replacements for it exist (Parcel v2, Webpack, Vite, etc.), which were examined and tested on a fresh Electron+React project. Vite seemed to be the most viable candidate, though, like the others, it also requires some refactoring beforehand. This issue will be used to track the migration progress.

Related tasks:

@eglitise eglitise added the chore Internal changes not visible to the user label May 9, 2024
@eglitise
Copy link
Collaborator Author

eglitise commented May 9, 2024

@idebenone about electron-vite: I checked it out, though I'm currently working with the other electron-vite (it's kinda odd how there's two of them). I've managed to get the Inspector's web version working, so now the issue is only with the Electron version.
Right now I see two main problems: clear separation of main and renderer process code (see the configs and shared folders), and conditional use of browser or Electron polyfills (not really sure of the best practices here).
Ideally, both problems should be resolved with the current Parcel v1, so that the changes are separated from the actual Vite migration PR.

@idebenone
Copy link

@eglitise This is officially supported by vite, even I was confused when I saw two different electron-vite.

@eglitise
Copy link
Collaborator Author

eglitise commented May 9, 2024

The reason why I used the other electron-vite is because the officially supported version reuses Vite's default config file (vite.config.js), whereas the unofficial one has its own file (electron.vite.config.js). What this means is I can use one config file to build the Inspector's browser version, and the other to build the Electron version. I don't know if it's possible to build both formats using the same config file - but if yes, that could be the way forward.

@idebenone
Copy link

idebenone commented May 10, 2024

Could you please share the vite and electron-vite configuration?

@eglitise
Copy link
Collaborator Author

I've been working on it in a separate branch of my fork: https://github.com/eglitise/appium-inspector/tree/move-to-vite

@idebenone
Copy link

I tried out with latest Electron version. Still there are errors but I think its a good progress in my opinion.

You could take a look here,
https://github.com/idebenone/appium-inspector/tree/vite-electron-migrate

@eglitise
Copy link
Collaborator Author

Awesome, thank you 🙏 does it also work without bumping Electron? We use spectron for running integration tests, and its current version is tied to Electron 13. They can be bumped together after the migration. (Arguably spectron is also deprecated, so another solution will be needed for Electron 17+)

@idebenone
Copy link

Here you go
electron 13.6.9 https://github.com/idebenone/appium-inspector/tree/vite-migrate

Issues:

  • Found errors with electron-log
  • ipcRenderer.on() is not function error in Root.jsx and Session.jsx

I haven't tested out the remaining functionalities, but the electron app is running normally.

@eglitise
Copy link
Collaborator Author

eglitise commented May 11, 2024

Thanks, I also updated my branch based on your changes. ipcRenderer seems to be working fine if building for CJS, and I also adjusted the i18next asset paths, so the translations are loaded. Currently there's still the following issues:

  • The same error with electron-log
  • Electron's remote module (wasn't sure how to import @electron/remote properly without errors, so just commented it out)
  • Splash screen images aren't scaled down by the imported CSS
  • Building browser version still requires commenting out things in polyfills/index.js. I'm not fully sure of the root cause, but the current behavior seems to be as follows:
    • Browser build with await import: works fine
    • Browser build with require(): require is not defined. Unsure if vite-plugin-require might help here.
    • Electron build with await import: Module format "cjs" does not support top-level await. I'm guessing this is because ESM support was only added in Electron 28.
    • Electron build with require(): Cannot find module './electron'. Really not sure about why this happens.

@eglitise eglitise added the help wanted Extra attention is needed label Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Internal changes not visible to the user help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants