refactor(FR-2874): replace codemirror with monaco in BAICodeEditor#7390
Merged
Conversation
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the workspace-level pnpm dependency resolution to avoid a known phantom-import issue with @codemirror/lang-jinja under enableGlobalVirtualStore, which can break Vite/esbuild resolution in this monorepo.
Changes:
- Adds a pnpm
overridesrule to force@codemirror/lang-jinjato resolve to^6.0.1(instead of 6.0.0) to avoid phantom imports.
yomybaby
force-pushed
the
fix/FR-2874-codemirror-lang-jinja
branch
from
1e40223 to
526ef4b
Compare
yomybaby
force-pushed
the
fix/FR-2874-codemirror-lang-jinja
branch
from
e2a03f1 to
568dbbc
Compare
The phantom-import build failure under enableGlobalVirtualStore (FR-2856) came from a transitively-pulled @codemirror/lang-jinja that isn't even used. Instead of pinning a fixed version, drop the only codemirror consumer (BAICodeEditor) and switch it to the project's existing Monaco wrapper. Monaco is already used elsewhere (VFolderTextFileEditorModal, ThemeJsonConfigModal), so this consolidates on a single editor stack and removes the entire @codemirror / @uiw editor dependency surface.
…ild only
In dev, vite's dep optimizer wraps `vite-plugin-node-polyfills/shims/buffer` into its own CJS-interop chunk while the plugin also injects `import __buffer_polyfill from '...'; globalThis.Buffer = ... __buffer_polyfill` at the top of every chunk that touches Buffer. The shim chunk ends up importing its own default export, causing a TDZ access on `__vite__cjsImport0_vitePluginNodePolyfills_shims_buffer` that blocks the entire app from booting.
Scoping `globals.Buffer` to `'build'` keeps the prod bundle behavior unchanged while letting dev pre-bundle the polyfill (via `include: ['buffer']`) for explicit `import { Buffer } from 'buffer'` consumers. No browser code in this repo references the Buffer global — only `backend.ai-client-node.ts`, which isn't part of the React bundle.
- Wrap Monaco editor with a 1px `token.colorBorder` border + `token.borderRadius` so the editor reads as a bounded field (matches the visual weight of antd inputs around it).
- Pad the loading Skeleton with `token.paddingContent{Horizontal,Vertical}` so the placeholder doesn't sit flush against the border before the editor mounts.
- Caller `style` still wins via spread, so consumers can override border or padding when needed.
PR #7390 removed '@codemirror/state' from pnpm-workspace.yaml overrides without refreshing pnpm-lock.yaml, so CI's pnpm install --frozen-lockfile failed with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. After rebasing onto main (picks up FR-2866 which disables gitBranchLockfile), regenerate the root lockfile to match the current overrides and remove the now-stale pnpm-lock.fix!fr-2874-codemirror-lang-jinja.yaml branch lockfile.
yomybaby
force-pushed
the
fix/FR-2874-codemirror-lang-jinja
branch
from
568dbbc to
aaa50b3
Compare
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
yomybaby
changed the title
Expand the inline comment to document the two pieces of context that aren't obvious from the code: (a) why `Buffer: true` deadlocks dev (vite optimizer self-import TDZ on the polyfill shim chunk), and (b) why scoping to 'build' is safe specifically in this repo — no app code references Buffer; the only surviving `Buffer.byteLength` call is in a Node-only branch of cross-fetch (pulled transitively by i18next-http-backend) that never executes in the browser. Notes that the polyfill itself is a candidate for full removal in a follow-up.
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 #7378 (FR-2874)
Summary
Make
pnpm devactually boot onmainagain, post-FR-2856 (enableGlobalVirtualStore). Two latent issues were exposed by the global virtual store; both are fixed here.1. codemirror → monaco (drop unused editor stack)
@codemirror/lang-jinja@6.0.0(pulled transitively via@uiw/codemirror-extensions-langs) imports@codemirror/state/@codemirror/viewas phantom deps. UnderenableGlobalVirtualStoreesbuild/Vite can't resolve them and dev fails:BAICodeEditorwas the only consumer of the codemirror stack, and the project already has a Monaco wrapper (loadMonacoEditor+@monaco-editor/react) used byVFolderTextFileEditorModalandThemeJsonConfigModal. RewriteBAICodeEditoron top of Monaco and drop the entire@codemirror/@uiweditor surface (-58 packages from the graph).Also wraps the Monaco editor with a
1px solid token.colorBorder(+token.borderRadius) so it reads as a bounded field, and pads the loading Skeleton withtoken.paddingContent{Horizontal,Vertical}.Removed from
react/package.json:@codemirror/lang-jinja@codemirror/lang-liquid@codemirror/language@uiw/codemirror-extensions-langs@uiw/react-codemirrorRemoved from
pnpm-workspace.yamloverrides:@codemirror/state@codemirror/lang-jinja(had been needed to dodge the phantom imports)2. `vite-plugin-node-polyfills` Buffer TDZ in dev
With the new dep graph, dev served the app but the browser crashed on load with:
```
chunk-XXXXXX.js Uncaught ReferenceError: Cannot access '__vite__cjsImport0_vitePluginNodePolyfills_shims_buffer' before initialization
```
`Buffer: true` makes the plugin prepend `import __buffer_polyfill from 'vite-plugin-node-polyfills/shims/buffer'; globalThis.Buffer = ... __buffer_polyfill` to every chunk that touches Buffer. Vite's dep optimizer also wraps the shim itself into a CJS-interop chunk — that chunk then ends up importing its own default export, producing the TDZ.
Scope `globals.Buffer` to `'build'`. Prod build behavior is unchanged; dev pre-bundles the polyfill (via `include: ['buffer']`) for explicit `import { Buffer } from 'buffer'` callers. No browser-side code in this repo references the Buffer global — only `src/lib/backend.ai-client-node.ts`, which isn't part of the React bundle.
Test plan