fix(desktop-electron): fix resource loading under file:// protocol#17125
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Electron renderer asset loading when the app is served via the file:// protocol (dev + production), ensuring public assets and entrypoints resolve correctly and adding regression tests to prevent future breakage.
Changes:
- Fix
publicDirresolution inelectron.vite.config.tsso Electron bundles/serves assets frompackages/app/public. - Convert renderer HTML asset references from absolute (
/…) to relative (./…) to work underfile://, and remove the web manifest link. - Add regression tests to validate HTML paths and
publicDirresolution.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/components/font.tsx | Skips font preloads under file:// to avoid double-loading due to credential-mode mismatch. |
| packages/desktop-electron/tsconfig.json | Excludes *.test.ts from the package’s TypeScript project build/typecheck. |
| packages/desktop-electron/src/renderer/loading.html | Switches asset/script references to relative paths; removes manifest link. |
| packages/desktop-electron/src/renderer/index.html | Switches asset/script references to relative paths; removes manifest link. |
| packages/desktop-electron/src/renderer/html.test.ts | Adds tests to enforce relative paths in renderer HTML and validate publicDir resolution. |
| packages/desktop-electron/electron.vite.config.ts | Corrects publicDir relative to root for renderer builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { fileURLToPath } from "node:url" | ||
|
|
||
| const dir = dirname(fileURLToPath(import.meta.url)) | ||
| const root = resolve(dir, "../..") |
There was a problem hiding this comment.
root is resolved as ../.. from src/renderer, which points to packages/desktop-electron/src. That makes join(root, "electron.vite.config.ts") look for src/electron.vite.config.ts (doesn't exist) and also causes resolve(root, rendererRoot, pub) to duplicate src in the path. Adjust the base directory so it points at packages/desktop-electron (e.g., walk up one more level or resolve from import.meta.url to the package root) before reading the config and resolving publicDir.
| const root = resolve(dir, "../..") | |
| const root = resolve(dir, "../../..") |
publicDir was resolved relative to renderer root (src/renderer/) not the config file, so no public assets were ever served or bundled. Also convert all HTML paths to relative, remove inapplicable web manifest link, and skip font preloads under file:// to avoid crossorigin credential mismatch.
4332484 to
34a55eb
Compare
Summary
publicDirresolution inelectron.vite.config.ts-- was../app/public(resolved relative to rendererroot: "src/renderer/"tosrc/app/public, doesn't exist). Changed to../../../app/publicwhich correctly resolves topackages/app/public. This has been broken since the initial electron commit (5cf235fa6), meaning public assets likeoc-theme-preload.js, favicons, and the web manifest were never served in dev or included in production builds../) inindex.htmlandloading.html-- absolute/paths resolve to filesystem root underfile://protocol (e.g.file:///C:/oc-theme-preload.json Windows).<link rel="manifest">from Electron HTML -- web manifests are a PWA concept, not applicable in Electron.file://--crossorigin="anonymous"on<link rel="preload">creates a credential mode mismatch with@font-face url()fetches underfile://, causing fonts to load twice.Visible impact
Users on non-default themes get a flash of wrong colors on every launch because
oc-theme-preload.js(which injects cached theme CSS before the app renders) was never bundled. Default theme users are unaffected (the script early-returns foroc-2).