fix(FR-2606): clear residual build-time warnings (BUI dts + module directive)#7113
Merged
graphite-app[bot] merged 1 commit intomainfrom Apr 30, 2026
Conversation
This was referenced Apr 28, 2026
Merged
Contributor
Author
3 tasks
Contributor
Coverage Report for backend-ai-ui-coverage (./packages/backend.ai-ui)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
58aa395 to
69d255c
Compare
e580e37 to
af61f26
Compare
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
Cleans up non-fatal build-time warnings surfaced after the Vite migration by fixing .d.ts emit typing issues, moving a misplaced React Compiler directive, and breaking a Rollup-detected circular dependency between backend.ai-ui components and hooks.
Changes:
- Fixes
backend.ai-uitype generation warnings by adding expliciti18ntyping, wiringvite/clientambient typings into both TS andvite-plugin-dts, and replacingprocess.env.NODE_ENVusage withimport.meta.env.PROD. - Moves a module-level
'use memo'directive into the correct component function body to avoid bundler directive warnings. - Breaks a components↔hooks cycle in
backend.ai-uiby replacing barrel-to-barrel imports with direct module imports and converting a value import toimport type.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| react/src/components/SwitchToProjectButton.tsx | Moves 'use memo' from module scope into the component function body. |
| react/src/components/STokenLoginBoundary.test.tsx | Converts Jest-style mocks to Vitest (vi.*) APIs. |
| react/src/ambient.d.ts | Adds vite/client type reference to type import.meta.env in React TS checks. |
| packages/backend.ai-ui/vite.config.ts | Ensures vite-env.d.ts is included in vite-plugin-dts processing. |
| packages/backend.ai-ui/vite-env.d.ts | Adds vite/client ambient typings for import.meta.env.*. |
| packages/backend.ai-ui/tsconfig.json | Includes vite-env.d.ts so TS picks up Vite ambient types during builds. |
| packages/backend.ai-ui/src/locale/index.ts | Adds explicit i18n instance type annotation to keep emitted .d.ts portable. |
| packages/backend.ai-ui/src/locale/.prettierrc | Removes per-directory Prettier config to restore parent config inheritance. |
| packages/backend.ai-ui/src/hooks/useBAISignedRequestWithPromise.ts | Replaces hooks→components barrel import with a direct provider-hook import. |
| packages/backend.ai-ui/src/hooks/useBAILogger.tsx | Switches environment gating to import.meta.env.PROD to avoid process usage. |
| packages/backend.ai-ui/src/hooks/index.ts | Removes components-barrel dependency by using direct imports and type-only imports. |
| packages/backend.ai-ui/src/components/provider/BAIMetaDataProvider/types.ts | Converts hooks import to import type to eliminate runtime dependency edges. |
| packages/backend.ai-ui/src/components/provider/BAIMetaDataProvider/hooks/useBAIDeviceMetaData.ts | Replaces hooks-barrel import with direct leaf-module import to avoid cycles. |
| packages/backend.ai-ui/src/components/provider/BAIClientProvider/hooks/useAnonymousBAIClient.ts | Replaces hooks-barrel import with direct leaf-module import to avoid cycles. |
| packages/backend.ai-ui/.prettierrc.json | Moves JSON-sort options into a scoped override for src/locale/*.json. |
af61f26 to
4111b2d
Compare
245b162 to
fa43a8c
Compare
9b8e590 to
f1bd608
Compare
fa43a8c to
9ab3c7b
Compare
f1bd608 to
72375aa
Compare
5dd3d34 to
c55de1c
Compare
72375aa to
467c14c
Compare
yomybaby
added a commit
that referenced
this pull request
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. - `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`.) - 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. - 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. - 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`. - 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. Sits on top of #7113 (FR-2606 residual build-time warnings). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
467c14c to
71899ca
Compare
c55de1c to
026732d
Compare
This was referenced Apr 30, 2026
Merge activity
|
…rective) (#7113) Resolves #6809(FR-2606) ## Summary Cleans up residual `pnpm run build` warnings introduced or surfaced by the Vite migration (FR-2611). All non-fatal in the original log (build was already exiting 0); cleared here as a follow-up to the FR-2606 series. Three classes of warnings are addressed: 1. **BUI dts emit type errors** (TS2742 + TS2591 + a follow-on TS2339) 2. **Module-level `'use memo'` directive** ignored by Rollup 3. **Components ↔ hooks circular dependency** in BUI, surfaced via `ResourceTypeIcon` re-export through the components barrel ## 1. BUI dts emit (`vite-plugin-dts`) — three TS diagnostics ### TS2742 — `i18n` inferred type (`packages/backend.ai-ui/src/locale/index.ts:92`) > The inferred type of 'i18n' cannot be named without a reference to '../../node_modules/i18next'. `createInstance(...)`'s inferred return type referenced `i18next` internal types, breaking portable `.d.ts` emit. Added explicit annotation: ```ts import { createInstance, type i18n as I18nInstance } from 'i18next'; export const i18n: I18nInstance = createInstance({ ... }); ``` ### TS2591 — `process` not defined (`packages/backend.ai-ui/src/hooks/useBAILogger.tsx:47`) BUI's `tsconfig.types` is intentionally `["vite-plugin-svgr/client"]` (no `node` types) because BUI is a browser-only library. The single `process.env.NODE_ENV` read violated that policy. Switched to Vite's first-class env API: ```ts private enabled: boolean = !import.meta.env.PROD; ``` ### TS2339 — `import.meta.env` not typed (follow-up to TS2591 fix) For `import.meta.env.PROD` to be typed in BUI's dts emit, `vite/client` ambient types had to be wired in. Three coordinated changes: - `packages/backend.ai-ui/vite-env.d.ts` — added `/// <reference types="vite/client" />`. The file already existed at the package root but was orphaned; included it so it actually gets picked up. - `packages/backend.ai-ui/tsconfig.json` — added `vite-env.d.ts` to `include`. - `packages/backend.ai-ui/vite.config.ts` — added `vite-env.d.ts` to the `dts` plugin's own `include` array (separate from `tsconfig.include`; this was the missing piece that made `import.meta.env.PROD` typed in dts emit context). - `react/src/ambient.d.ts` — added the same reference for the `react/` tsc check (verify.sh) which compiles BUI source through the path alias. ## 2. Module-level `'use memo'` (`react/src/components/SwitchToProjectButton.tsx:1`) > Module level directives cause errors when bundled, "use memo" in "..." was ignored. Moved the directive from module level to the inner function body (`SwitchToProjectButtonContent`), matching `.claude/rules/react-compiler-memoization.md` convention. The wrapper component (`SwitchToProjectButton`) renders only Suspense + child and does not need its own memo directive. ## 3. BUI components ↔ hooks circular dependency Rollup chunk-graph warning: > Export "ResourceTypeIcon" of module ".../BAIResourceNumberWithIcon.tsx" was reexported through module ".../components/index.ts" while both modules are dependencies of each other... The actual cycle traced: ``` components/index.ts (barrel) ↓ re-exports BAIResourceNumberWithIcon BAIResourceNumberWithIcon.tsx ↓ imports './provider' (provider barrel) provider barrel ↓ exports BAIMetaDataProvider, BAIClientProvider hooks BAIMetaDataProvider/hooks/useBAIDeviceMetaData.ts:1 BAIClientProvider/hooks/useAnonymousBAIClient.ts:1 BAIMetaDataProvider/types.ts:1 ↓ each imports from '../../../[..]/hooks' (hooks barrel) hooks/index.ts:1-5 ↓ imports value names from '../components' 🔴 cycle closes ``` The cycle was reachable through three back-edges from the `components/provider/...` subtree into the hooks barrel, plus the hooks barrel's own value imports back into the components barrel. Six coordinated edits broke it: - `BUI/src/hooks/index.ts:1-5` — barrel-to-barrel import replaced with three direct module imports + one `import type` (for the type-only `ResourceSlotName`). - `BUI/src/hooks/useBAISignedRequestWithPromise.ts:1` — same pattern; barrel → direct module path. - `BUI/src/components/provider/BAIMetaDataProvider/hooks/useBAIDeviceMetaData.ts:1` — barrel → direct module path on the hooks side. - `BUI/src/components/provider/BAIClientProvider/hooks/useAnonymousBAIClient.ts:1` — same pattern. - `BUI/src/components/provider/BAIMetaDataProvider/types.ts:1` — value-style import of a type promoted to `import type`. Net structural change: the BUI hooks barrel no longer imports anything from the components barrel, and the surviving `provider/* → hooks` edges all go through direct module paths whose target files have no further cycle exposure. Components-subtree files are still free to use the hooks barrel; the dependency arrow is now one-way only. ## Locale Prettier config — fixed parent override (in earlier amend) `packages/backend.ai-ui/src/locale/.prettierrc` was a per-directory override containing only the JSON-sort plugin, with no other options set. Prettier's config resolution picks the nearest config file and does NOT cascade from parents, so `singleQuote` defaulted to `false` for files in `src/locale/` — including `index.ts`, which kept getting double-quoted whenever lint-staged ran. Fix: deleted `src/locale/.prettierrc` and moved the JSON-sort options into the parent BUI `.prettierrc.json` as an `overrides` entry scoped to `src/locale/*.json`. The directory now inherits the rest of the BUI Prettier config (including `singleQuote: true`) while retaining the JSON-only sort behavior. ## Verification - [x] `bash scripts/verify.sh` — Relay / Lint / Format / TypeScript all PASS - [x] `pnpm run build` — exit 0; the following diagnostics are no longer present in the log: - TS2742 (`i18n` portable) - TS2591 (`process` not defined) - TS2339 (`import.meta.env.PROD` not typed) - "Module level directives cause errors" (`'use memo'`) - "reexported through ... while both modules are dependencies of each other" (`ResourceTypeIcon` cycle) Remaining build warnings (5 mixed dynamic+static imports, 2 chunks > 2 MB, several runtime-resolved CSS/manifest assets) are out of scope and tracked separately. ## Stack Sits on top of #7104 (FR-2606 Monaco self-hosting). Top of the Vite migration follow-up stack.
026732d to
be64c51
Compare
71899ca to
5adac3e
Compare
graphite-app Bot
pushed a commit
that referenced
this pull request
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_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).
Base automatically changed from
04-27-fix_fr-2606_restore_monaco_self-hosting_in_vite_dev_server
to
main
April 30, 2026 12:38
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
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.

Resolves #6809(FR-2606)
Summary
Cleans up residual
pnpm run buildwarnings introduced or surfaced by the Vite migration (FR-2611). All non-fatal in the original log (build was already exiting 0); cleared here as a follow-up to the FR-2606 series.Three classes of warnings are addressed:
'use memo'directive ignored by RollupResourceTypeIconre-export through the components barrel1. BUI dts emit (
vite-plugin-dts) — three TS diagnosticsTS2742 —
i18ninferred type (packages/backend.ai-ui/src/locale/index.ts:92)createInstance(...)'s inferred return type referencedi18nextinternal types, breaking portable.d.tsemit. Added explicit annotation:TS2591 —
processnot defined (packages/backend.ai-ui/src/hooks/useBAILogger.tsx:47)BUI's
tsconfig.typesis intentionally["vite-plugin-svgr/client"](nonodetypes) because BUI is a browser-only library. The singleprocess.env.NODE_ENVread violated that policy. Switched to Vite's first-class env API:TS2339 —
import.meta.envnot typed (follow-up to TS2591 fix)For
import.meta.env.PRODto be typed in BUI's dts emit,vite/clientambient types had to be wired in. Three coordinated changes:packages/backend.ai-ui/vite-env.d.ts— added/// <reference types="vite/client" />. The file already existed at the package root but was orphaned; included it so it actually gets picked up.packages/backend.ai-ui/tsconfig.json— addedvite-env.d.tstoinclude.packages/backend.ai-ui/vite.config.ts— addedvite-env.d.tsto thedtsplugin's ownincludearray (separate fromtsconfig.include; this was the missing piece that madeimport.meta.env.PRODtyped in dts emit context).react/src/ambient.d.ts— added the same reference for thereact/tsc check (verify.sh) which compiles BUI source through the path alias.2. Module-level
'use memo'(react/src/components/SwitchToProjectButton.tsx:1)Moved the directive from module level to the inner function body (
SwitchToProjectButtonContent), matching.claude/rules/react-compiler-memoization.mdconvention. The wrapper component (SwitchToProjectButton) renders only Suspense + child and does not need its own memo directive.3. BUI components ↔ hooks circular dependency
Rollup chunk-graph warning:
The actual cycle traced:
The cycle was reachable through three back-edges from the
components/provider/...subtree into the hooks barrel, plus the hooks barrel's own value imports back into the components barrel. Six coordinated edits broke it:BUI/src/hooks/index.ts:1-5— barrel-to-barrel import replaced with three direct module imports + oneimport type(for the type-onlyResourceSlotName).BUI/src/hooks/useBAISignedRequestWithPromise.ts:1— same pattern; barrel → direct module path.BUI/src/components/provider/BAIMetaDataProvider/hooks/useBAIDeviceMetaData.ts:1— barrel → direct module path on the hooks side.BUI/src/components/provider/BAIClientProvider/hooks/useAnonymousBAIClient.ts:1— same pattern.BUI/src/components/provider/BAIMetaDataProvider/types.ts:1— value-style import of a type promoted toimport type.Net structural change: the BUI hooks barrel no longer imports anything from the components barrel, and the surviving
provider/* → hooksedges all go through direct module paths whose target files have no further cycle exposure. Components-subtree files are still free to use the hooks barrel; the dependency arrow is now one-way only.Locale Prettier config — fixed parent override (in earlier amend)
packages/backend.ai-ui/src/locale/.prettierrcwas a per-directory override containing only the JSON-sort plugin, with no other options set. Prettier's config resolution picks the nearest config file and does NOT cascade from parents, sosingleQuotedefaulted tofalsefor files insrc/locale/— includingindex.ts, which kept getting double-quoted whenever lint-staged ran.Fix: deleted
src/locale/.prettierrcand moved the JSON-sort options into the parent BUI.prettierrc.jsonas anoverridesentry scoped tosrc/locale/*.json. The directory now inherits the rest of the BUI Prettier config (includingsingleQuote: true) while retaining the JSON-only sort behavior.Verification
bash scripts/verify.sh— Relay / Lint / Format / TypeScript all PASSpnpm run build— exit 0; the following diagnostics are no longer present in the log:i18nportable)processnot defined)import.meta.env.PRODnot typed)'use memo')ResourceTypeIconcycle)Remaining build warnings (5 mixed dynamic+static imports, 2 chunks > 2 MB, several runtime-resolved CSS/manifest assets) are out of scope and tracked separately.
Stack
Sits on top of #7104 (FR-2606 Monaco self-hosting). Top of the Vite migration follow-up stack.