fix(renderer): fall back on the bridge, not on window - #79
Merged
Conversation
`ipc.ts` guarded on `typeof window !== 'undefined'` and then read `window.electronAPI`, so outside Electron — browser-mode tests, Storybook — it took the first branch and left `electronAPI` undefined. The `createRPCClient` call one line down then threw while the module was still evaluating, which took down every module that transitively imports it, at import time, before any test body ran. That is how #75's browser suite shipped red: `markdown-renderer-security.test.tsx` failed with `Cannot read properties of undefined (reading 'invoke')` and nothing noticed, because fork-ci.yml runs node/main-db/migrations/scripts and not the browser project. Guarding on the bridge itself makes the unavailable-channel errors below reachable, which is what they were written for. The suite also needed the ipc stub every other renderer test installs: importing the markdown renderer constructs the SSH connection store, whose Resources call RPC from their constructor, and those rejections failed the file even with both assertions passing. Full workspace suite now green: 398 files, browser project included.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
src/renderer/lib/ipc.tsguarded ontypeof window !== 'undefined'and then readwindow.electronAPI. Outside Electron — browser-mode tests, Storybook —windowexistswhile the preload bridge does not, so it took the first branch, left
electronAPIundefined, and the
createRPCClient(electronAPI.invoke)call one line below threw whilethe module was still evaluating. Anything transitively importing it failed to load at all,
before any test body ran. Guarding on the bridge itself makes the unavailable-channel
errors written just underneath actually reachable.
This is not hypothetical: #75 landed a browser suite that was red from the first commit —
markdown-renderer-security.test.tsxdied withCannot read properties of undefined (reading 'invoke'). Nothing caught it because.github/workflows/fork-ci.ymlruns thenode, main-db, migrations and scripts projects, not the browser one, and the author had no
local Chromium.
The suite also needed the ipc stub every other renderer test installs: importing the
markdown renderer transitively constructs the SSH connection store, whose
Resources callRPC from their constructor, and those rejections failed the file even with both assertions
passing.
In packaged Electron the preload bridge is always present, so the new fallback never fires
there.
Related issues
Follow-up to #75.
Testing
pnpm run test(repo root) — 398 files, all 9 projects, browser project included.Before this change the same command failed with
1 failed | 397 passed.pnpm typecheck— app + release scriptspnpm exec oxlint .pnpm run formatpnpm exec vitest run --project browser src/renderer/tests/browser/markdown-renderer-security.test.tsx— 2 passed, 0 errors (was: failed to import)
Screenshot/Recording (if applicable)
Not applicable.
Checklist