Skip to content

Conversation

@maximsmol
Copy link

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

Electron now supports ESM by settings "type": "module" in package.json but @electron-forge/plugin-vite will output CommonJS code regardless, which will then fail to load (typically by trying to reference require).

This patch reads type from package.json and automatically changes the main vite config output format to esm if modules are requested. Note that we parse package.json twice now (the first time to look for a forge config) but this seemed cleaner than trying to pass the parsed JSON around.

I am deliberately leaving preload scripts as-is since they ignore "type": "module" and load fine with the current settings. That being said, Electron supports ESM preload scripts, so we might want to automatically change the output format there as well. This would also require renaming the output from .js to .mjs There is also the sandboxing limitation, which may mean that bundling to CommonJS is the best way for preload scripts regardless, but I do not understand well enough how people use preload scripts (and in particular how common sandboxing is) to make that decision.


Please let me know if I need to add docs or automated tests here—the change is pretty trivial so I did not bother.

Related: #3684

@maximsmol maximsmol requested a review from a team as a code owner October 6, 2025 22:27
@erickzhao erickzhao changed the title feat: plugin-vite: automatically switch to esm mode when enabled feat(plugin-vite): automatically switch to esm mode when enabled Oct 6, 2025
@erickzhao erickzhao self-requested a review October 6, 2025 22:48
@maximsmol maximsmol force-pushed the maximsmol/vite-auto-esm branch from 87cce28 to 35082c7 Compare October 7, 2025 04:10
Copy link
Member

@BlackHole1 BlackHole1 left a comment

Choose a reason for hiding this comment

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

@BlackHole1 BlackHole1 self-requested a review October 10, 2025 01:40
@maximsmol
Copy link
Author

Could we move on this? Are there any blockers?

Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this file? Will the absence of this file cause test failures?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. I could make it optional but IIRC there is some other code that actually relies on there always being a package.json anyway, except that it's not hit by the tests so the fixture worked before

@BlackHole1 BlackHole1 force-pushed the maximsmol/vite-auto-esm branch from 11adf77 to 19e674c Compare November 25, 2025 03:20
@BlackHole1 BlackHole1 self-requested a review November 25, 2025 03:20
Copy link
Member

@erickzhao erickzhao left a comment

Choose a reason for hiding this comment

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

Hey @maximsmol, thanks for the PR. I recall having some offline discussion about this, and will get back with a review soon.

Please note that you have to get your commits signed for any contributions to Electron!

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.

3 participants