Repository navigation
fix(desktop): keep the browser pane chunk parseable - #60388
multiserviciossv wants to merge 3 commits into
Conversation
github-actions
Bot
added
the
needs:compliance
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix looks sound and does what it says. I built the desktop main process (bunx electron-vite build) on the base and with this PR, then ran node --check on all 650 out/main chunks. On the base only the browser pane-*.js chunk fails, with Identifier 'n' has already been declared. With the PR every chunk passes and none has two createRequire calls. electron.vite.config.test.ts passes. I didn't open the Electron app itself.
I also checked that the commented-out shim is needed. electron-vite's vite:esm-shim skips a chunk only when the code contains its exact shim text, and it sees the banner before minification removes the comment. With the comment taken out, the build adds a second const require to many chunks. So this depends on electron-vite's shim text staying the same, but the current config already relies on that.
Public API: unchanged (desktop build config only).
No blocking concerns.
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
The new commit only merges dev in, and the fix itself is unchanged. I rebuilt the desktop main process at the new head and ran node --check on all 650 out/main chunks: all pass, including the browser pane-* chunk. electron.vite.config.test.ts passes.
Public API: unchanged (desktop build config only).
No new concerns.
There was a problem hiding this comment.
The new commit only merges dev in, and the fix itself is unchanged. I rebuilt the desktop main process at the new head and ran node --check on all 650 out/main chunks: all pass, including the browser pane-* chunk. electron.vite.config.test.ts passes.
Public API: unchanged (desktop build config only).
No new concerns.
|
Superseded by #60430 (merged in v2.2.4). That change removes the banner and disables electron-vite's esm-shim, so Rolldown's scoped createRequire runtime is used. It fixes the same browser pane collision and adds bundle verification. #60369 is already closed. Closing this to avoid a duplicate fix; thanks for the reviews. |
Issue for this PR
Closes #60369
Fixes #60386
Related: #60023
Type of change
What does this PR do?
The desktop main build produces a browser pane chunk that cannot be parsed in 2.2.2. The fixed CommonJS banner declares a module-scoped
require, while the bundled@effect/platform-nodeinternal/httpshim declares its own top-levelrequire. The minifier renames both ton, so the lazypane-*.jschunk dies withIdentifier 'n' has already been declaredand the browser panel never attaches.This keeps
requireoff the module scope by assigning it onglobalThis(preserving an existing Electronrequire), and keeps the exact electron-vite shim in a comment so electron-vite'svite:esm-shimplugin still detects it and does not inject a second copy.The fix is desktop build config only; the public API is unchanged.
How did you verify your code works?
cd packages/desktop && bunx electron-vite buildnode --checkon everyout/main/*.js(650 chunks). It fails onpane-*.jsbefore the change and passes after.bun test electron.vite.config.test.tsbun run typecheckThe failure reproduces on Linux from the v2.2.2 sources, so it is not Windows-only. I did not open the Electron app itself.
Checklist
Added by the opencode bot (AI-generated).
Fixes #60386
Related: #60023