Skip to content

fix(FR-2607): align Electron publicPath patch with Vite output#7189

Merged
graphite-app[bot] merged 1 commit intomainfrom
04-30-fix_fr-2607_align_electron_publicpath_patch_with_vite_output
Apr 30, 2026
Merged

fix(FR-2607): align Electron publicPath patch with Vite output#7189
graphite-app[bot] merged 1 commit intomainfrom
04-30-fix_fr-2607_align_electron_publicpath_patch_with_vite_output

Conversation

@yomybaby
Copy link
Copy Markdown
Member

@yomybaby yomybaby commented Apr 30, 2026

Resolves the gap between PR #6871 (Vite spike for es6:// publicPath via
renderBuiltUrl) and PR #6765 / FR-2612 (single React build + post-patch
policy). The post-patch script was still searching for CRA-era /static/...
paths after the Vite migration emitted /assets/..., so make dep produced
an Electron staging dir that referenced root-relative /assets/... that
Electron could not serve via the es6:// scheme handler.

Verification (PR #7113 head, this fix applied)

  • bash scripts/verify.sh → Relay / Lint / Format / TypeScript ALL PASS
  • make clean && make dep → "Patched 2 file(s). Verification passed: index.html contains es6://assets/"
  • make dep run twice → idempotent skip ("Electron app already prepared, skipping")
  • pnpm run electron:d → renderer spawns with es6:// scheme registered, login UI renders, all stylesheets load (rule counts > 0), body bg = rgb(247, 247, 246), did-fail-load / webRequest err / 404s = 0
  • make mac_arm64compile_localproxy + electron-packager (.app) produced cleanly. (DMG step fails on a local NODE_MODULE_VERSION mismatch in a globally-installed electron-installer-dmg; unrelated to this fix and does not affect CI which uses npx.)

Changes

scripts/patch-electron-publicpath.js

  • Replace CRA-era patch patterns (/static/js, /static/css, asset-manifest.json, webpack runtime .p='/' rewrites) with Vite-aware patterns.
  • Patch index.html: rewrite ="/assets/="es6://assets/ on src/href attributes only.
  • Patch CSS files under assets/: url(/assets/...)url(es6://assets/...) in all quote styles.
  • Drop dead Webpack-specific code paths (asset-manifest.json, static/{js,css}, webpack runtime publicPath assignment) that no longer exist in Vite output.
  • Update verification marker from es6://static/js/main to es6://assets/.
  • Leave <base href="/"> untouched: the renderer loads index.html via file://, and rewriting <base> to es6:// would break window.location/origin alignment (history API, same-origin checks). Webpack-era patch script also did not touch <base>.
  • Verification failure now exits non-zero so the Makefile recipe aborts instead of silently continuing.

Makefile

  • Update dep_electron idempotency marker from es6://static/js/main to es6://assets/ to match the new patch script.
  • Replace ; chaining with && between the dep_electron commands so a failed patch script aborts the recipe.
  • Update inline comment that referenced the old marker.

react/vite.config.ts

react/package.json

  • Remove three unused CRA-era devDependencies: @svgr/webpack (Vite uses vite-plugin-svgr), react-scripts (CRA core, retired in FR-2611), react-dev-utils (no imports anywhere). All three were verified to have zero references in source.

Stack

Sits on top of #7113 (FR-2606 residual build-time warnings).

@github-actions github-actions Bot added the size:L 100~500 LoC label Apr 30, 2026
Copy link
Copy Markdown
Member Author

yomybaby commented Apr 30, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@yomybaby yomybaby marked this pull request as ready for review April 30, 2026 11:25
Copilot AI review requested due to automatic review settings April 30, 2026 11:25
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

Coverage Report for root-coverage

Status Category Percentage Covered / Total
🔵 Lines 3.78% 85 / 2247
🔵 Statements 4% 92 / 2299
🔵 Functions 3.87% 14 / 361
🔵 Branches 5.12% 68 / 1327
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
scripts/patch-electron-publicpath.js 0% 0% 0% 0% 26-109
Generated in workflow #114 for commit f6c26dc by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

Coverage Report for react-coverage (./react)

Status Category Percentage Covered / Total
🔵 Lines 6.89% 1781 / 25826
🔵 Statements 5.79% 1976 / 34105
🔵 Functions 5.35% 296 / 5523
🔵 Branches 4.11% 1291 / 31365
File CoverageNo changed files found.
Generated in workflow #114 for commit f6c26dc by the Vitest Coverage Report Action

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Aligns the Electron staging “publicPath” post-patch flow with Vite’s build output (/assets/...) so Electron can correctly load bundled assets via the custom es6:// scheme without requiring a second, Electron-specific Vite build.

Changes:

  • Update the Electron post-patch script to rewrite Vite-style asset references (/assets/...) in index.html and emitted CSS.
  • Adjust dep_electron idempotency marker + failure propagation in Makefile to match the new patch behavior.
  • Remove the now-dead Electron-only renderBuiltUrl branch from react/vite.config.ts and drop unused CRA-era devDependencies from react/package.json.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
scripts/patch-electron-publicpath.js Switch patching logic from CRA /static/... to Vite /assets/... and make verification fail fast.
Makefile Update Electron prep skip-marker and ensure the patch step aborts the recipe on failure.
react/vite.config.ts Remove unused Electron-specific Vite build branch; rely on single web build + post-patch.
react/package.json Remove unused CRA-era devDependencies no longer referenced after Vite migration.

Comment thread scripts/patch-electron-publicpath.js Outdated
Comment thread scripts/patch-electron-publicpath.js Outdated
@yomybaby yomybaby changed the base branch from 04-28-fix_fr-2606_clear_residual_build-time_warnings_bui_dts_module_directive_ to graphite-base/7189 April 30, 2026 11:56
@yomybaby yomybaby force-pushed the graphite-base/7189 branch from 467c14c to af6555a Compare April 30, 2026 11:57
@yomybaby yomybaby changed the base branch from graphite-base/7189 to main April 30, 2026 11:57
@yomybaby yomybaby changed the base branch from main to 04-28-fix_fr-2606_clear_residual_build-time_warnings_bui_dts_module_directive_ April 30, 2026 12:01
@yomybaby yomybaby force-pushed the 04-30-fix_fr-2607_align_electron_publicpath_patch_with_vite_output branch from c72fccf to 7bee5af Compare April 30, 2026 12:08
@yomybaby yomybaby force-pushed the 04-28-fix_fr-2606_clear_residual_build-time_warnings_bui_dts_module_directive_ branch from 467c14c to 71899ca Compare April 30, 2026 12:08
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Apr 30, 2026

Merge activity

Resolves the gap between PR #6871 (Vite spike for `es6://` publicPath via
`renderBuiltUrl`) and PR #6765 / FR-2612 (single React build + post-patch
policy). The post-patch script was still searching for CRA-era `/static/...`
paths after the Vite migration emitted `/assets/...`, so `make dep` produced
an Electron staging dir that referenced root-relative `/assets/...` that
Electron could not serve via the `es6://` scheme handler.

## Verification (PR #7113 head, this fix applied)

- `bash scripts/verify.sh` → Relay / Lint / Format / TypeScript ALL PASS
- `make clean && make dep` → "Patched 2 file(s). Verification passed: index.html contains es6://assets/"
- `make dep` run twice → idempotent skip ("Electron app already prepared, skipping")
- `pnpm run electron:d` → renderer spawns with `es6://` scheme registered, login UI renders, all stylesheets load (rule counts > 0), body bg = rgb(247, 247, 246), `did-fail-load` / `webRequest err` / 404s = 0
- `make mac_arm64` → `compile_localproxy` + `electron-packager` (.app) produced cleanly. (DMG step fails on a local `NODE_MODULE_VERSION` mismatch in a globally-installed `electron-installer-dmg`; unrelated to this fix and does not affect CI which uses `npx`.)

## Changes

### `scripts/patch-electron-publicpath.js`
- Replace CRA-era patch patterns (`/static/js`, `/static/css`, `asset-manifest.json`, webpack runtime `.p='/'` rewrites) with Vite-aware patterns.
- Patch `index.html`: rewrite `="/assets/` → `="es6://assets/` on `src`/`href` attributes only.
- Patch CSS files under `assets/`: `url(/assets/...)` → `url(es6://assets/...)` in all quote styles.
- Drop dead Webpack-specific code paths (`asset-manifest.json`, `static/{js,css}`, webpack runtime `publicPath` assignment) that no longer exist in Vite output.
- Update verification marker from `es6://static/js/main` to `es6://assets/`.
- Leave `<base href="/">` untouched: the renderer loads `index.html` via `file://`, and rewriting `<base>` to `es6://` would break `window.location`/origin alignment (history API, same-origin checks). Webpack-era patch script also did not touch `<base>`.
- Verification failure now exits non-zero so the Makefile recipe aborts instead of silently continuing.

### `Makefile`
- Update `dep_electron` idempotency marker from `es6://static/js/main` to `es6://assets/` to match the new patch script.
- Replace `;` chaining with `&&` between the `dep_electron` commands so a failed patch script aborts the recipe.
- Update inline comment that referenced the old marker.

### `react/vite.config.ts`
- Remove the `BUILD_TARGET=electron` + `experimental.renderBuiltUrl` branch (PR #6871 spike). FR-2612 mandates a single web build with post-patch; the `renderBuiltUrl` path would require a second `vite build` per release and was never wired into the Makefile, making it dead code with regression risk if anyone toggled `BUILD_TARGET=electron`.
- Drop the now-unused `command` parameter from `defineConfig`.

### `react/package.json`
- Remove three unused CRA-era devDependencies: `@svgr/webpack` (Vite uses `vite-plugin-svgr`), `react-scripts` (CRA core, retired in FR-2611), `react-dev-utils` (no imports anywhere). All three were verified to have zero references in source.

## Stack

Sits on top of #7113 (FR-2606 residual build-time warnings).
@graphite-app graphite-app Bot force-pushed the 04-28-fix_fr-2606_clear_residual_build-time_warnings_bui_dts_module_directive_ branch from 71899ca to 5adac3e Compare April 30, 2026 12:21
@graphite-app graphite-app Bot force-pushed the 04-30-fix_fr-2607_align_electron_publicpath_patch_with_vite_output branch from 2a0e6d6 to f6c26dc Compare April 30, 2026 12:21
Base automatically changed from 04-28-fix_fr-2606_clear_residual_build-time_warnings_bui_dts_module_directive_ to main April 30, 2026 12:39
@graphite-app graphite-app Bot merged commit f6c26dc into main Apr 30, 2026
8 checks passed
@graphite-app graphite-app Bot deleted the 04-30-fix_fr-2607_align_electron_publicpath_patch_with_vite_output branch April 30, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants