feat(vite): support node integration in renderers - #4317
Conversation
Co-authored-by: rafael81 <36774+rafael81@users.noreply.github.com>
b681b12 to
b1cd0fc
Compare
|
Verification for this PR, since it has been open a while with no CI run — posting evidence rather Tests pass, and they are real controls. I ran the plugin's suite locally and then mutation-tested
Worth flagging honestly: the first mutant is caught by only one test — 6 of 7 still passed with One failure I want to attribute correctly: On the CI status. Three Relationship to #4258. That PR (@rafael81) came first and this one builds on it — it keeps the Happy to rebase, split anything out, or adjust the approach on request. |
`electronExportNames` omitted `ServiceWorkerMain`, so a renderer importing it
failed the build with
"ServiceWorkerMain" is not exported by "<virtual>:electron"
which is the same error Rollup gives for a name that does not exist at all --
a real API and a typo are indistinguishable.
The name is missing because the obvious source of truth is wrong here: in
`electron.d.ts` `ServiceWorkerMain` is declared only as a `type` inside the
`CrossProcessExports` namespace, while Electron 39.2.6's main process really
does export it as a constructor (`typeof require('electron').ServiceWorkerMain
=== 'function'`). Deriving the list from the typings drops it. The comment now
records that the runtime, not the typings, is what this list has to track.
The added spec generates one import per documented API and asserts the build
resolves. Verified as a discriminator, not just coverage: with the name present
the build succeeds, and with it removed the same spec fails on exactly the error
above.
Also verified against a real Electron 39.2.6 renderer (`nodeIntegration: true`,
`contextIsolation: false`), where the shim's `ipcRenderer` and `clipboard` are
live objects, `node:fs` reads a real file, and `app` is `undefined` -- matching
plain `require('electron')` in a renderer, which exports only the eight
renderer-side APIs.
Change-Id: I2b4088a5bea80d129127ae456758d818c948f297
Assisted-by: Claude (Anthropic)
Five defects, all invisible on Vite 6 and all silent on Vite 8.
1. `output.freeze` is Rollup-only. Vite 8 bundles Rolldown, whose
`OutputOptions` has no such key, so setting it is a type error
(3x TS2339/TS2353) and a no-op. Rolldown never emits `Object.freeze`
anywhere, so the opt-out is unnecessary there rather than merely
unsupported; gate it on `vite.rolldownVersion`.
2. `resolveId` ignored its `importer`, so the shim's own
`require("electron")` was re-claimed by the plugin and the virtual module
resolved to itself:
init_x = __esmMin(() => { moduleValue = (init_x(), ...) })
`__esmMin`'s `fn = 0` guard swallows the self-call, so instead of
recursing it yields `undefined` for every Electron export. Marking
shim-internal requests external is what keeps a real `require` in the
output -- simply declining them makes Rolldown resolve `electron` to the
npm package, which outside Electron is the *installer stub*, bundling
`getElectronPath()` and a "Downloading Electron binary..." branch into the
renderer with `fs`/`child_process` stubbed to `module.exports = {}`.
3. The shim called `require` through an alias
(`const runtimeRequire = require`). Rolldown only rewrites syntactically
direct `require(...)` calls into its external-module interop; the aliased
form is dropped. Call `require` directly.
4. `sharedTexture` was missing from `electronExportNames` -- a second
instance of the `ServiceWorkerMain` bug. It is declared as a `const` in
`CrossProcessExports`, so `MISSING_EXPORT` breaks the build for anyone
importing it. Found by the export-list spec, which is what it is for.
5. The specs asserted a literal `runtimeRequire(...)` and `freeze: false` --
Rollup's output shape rather than the behaviour. Assert the requested
specifier plus a `require` mention (Rolldown reaches it via
`require.apply(this, arguments)`, which a literal `require(` pattern
cannot match), and branch the freeze assertion on the bundler so the spec
keeps its teeth on Vite 6/7 instead of being loosened for both.
Also fixes a pre-existing lint error on this branch: the export-list spec
resolved `electron` as a bare specifier, but it is a devDependency of the
workspace root, not of this package, so `n/no-extraneous-require` rejected
it. The rule keys on the specifier, so `require.resolve(..., { paths })`
does not satisfy it; the typings are now located by path.
Verified in both directions, and the two version-conditional assertions were
mutation-checked so the branching did not turn them into no-ops:
Vite 8.0.3 / Rolldown 1.0.0-rc.12 (on `next`, merged with electron#4352):
tsc -b packages 0 errors
vitest --project fast 50/50 pass
Vite 6.4.3 / Rollup (this branch's base):
tsc -b packages/plugin/vite 0 errors
vitest --project fast 20/20 pass
eslint 0 problems
Mutants killed: forcing the freeze gate off fails "keeps user dependency
and Rollup settings"; pointing the typings path at a missing file fails
"re-exports every Electron API in the shipped export list" with ENOENT.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Closes #3961.
This follows up on #4258 and keeps its renderer-level
nodeIntegrationAPI. @rafael81 is retained as a co-author.Why externalization is not enough
While validating #4258 in Electron, I found that externalizing Electron and Node.js built-ins leaves bare ESM imports in the renderer output. Vite loads that output as a browser module, which fails before application code runs:
The development server similarly resolves Node.js modules to Vite browser-external stubs.
What changed
renderer[].nodeIntegrationsettingfs-extracan augment Node.js modulesThe runtime-shim approach follows the model proven by
vite-plugin-electron-renderer,implemented here only for built-ins so Forge does not gain another runtime
dependency.
Security boundary
This option only configures Vite. The corresponding
BrowserWindowmust usenodeIntegration: trueandcontextIsolation: false. It should only load trusted local content; a preload script withcontextBridgeremains the safer default.Validation
I also ran an Electron 39.2.6 / Vite 6.4.3 smoke fixture against the compiled Forge plugin in both development-server and production-build modes. It exercised default, named, namespace, side-effect, and dynamic built-in imports plus a transitive
fs-extraCommonJS dependency; both modes delivered the expected IPC result.If
nextis the right base for this, a rebased branch is ready:tzh476:feat/vite-node-integration-next— the same change re-applied onto a fresh copy of
next(1 commit ahead, 0 behind, the same 6 files).I noticed every recently merged
feat*PR targetsnextrather thanmain, so this may be aimed at thewrong branch. Rather than change the base ref here — which caused unrelated diff churn on #4168 — I
re-applied the commits onto
next, which is what was asked for there. Two things needed adapting:vite.renderer.config.tskeepsnext's./vite.base.config.jsimport convention, and the README keepsboth the existing "Main process hot restart" section and the new one.
Say the word and I will open it as a PR, or close this one in its favour — whichever you prefer. Happy to
leave it on
maintoo if that is correct.