Skip to content

chore(frontend): upgrade frontend to monaco-languageclient v10 - #4997

Merged
aglinxinyuan merged 51 commits into
apache:mainfrom
aglinxinyuan:chore/monaco-lsp-v10
Jul 5, 2026
Merged

chore(frontend): upgrade frontend to monaco-languageclient v10#4997
aglinxinyuan merged 51 commits into
apache:mainfrom
aglinxinyuan:chore/monaco-lsp-v10

Conversation

@aglinxinyuan

@aglinxinyuan aglinxinyuan commented May 9, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Brings the frontend's Monaco / VS Code language-client stack up to monaco-languageclient v10 (from v8.8.3) and the matching @codingame/monaco-vscode-* v25.1.2 ecosystem. v10 split the old single-wrapper API into three composable pieces (MonacoVscodeApiWrapper + EditorApp + LanguageClientWrapper), so the editor component is rewritten and the build/test pipeline picks up the codingame v25 quirks the new packages introduce. Adjacent dep cleanups land here too — several codingame side-deps and the R UDF editor become superfluous once v10 is in place.

API shape: v8 → v10

v8 (before)                                      v10 (after)
─────────────────                                ──────────────────────────────────
new MonacoEditorLanguageClientWrapper(           const api  = new MonacoVscodeApiWrapper(...)
  // single config with editor + LSP + workers   const app  = new EditorApp(...)
  // all in one bag                              const lc   = new LanguageClientWrapper(...)
).start(htmlContainer)                           await api.start();         // global vscode-api init
                                                 await app.start(container);// mount editor
                                                 await lc.start();          // open WebSocket to LS

The editor component is rewritten against the new shape, plus a whenReady() handshake on the default language extensions and a post-mount forceTokenization loop to fix a TextMate first-paint race that otherwise renders every token as the default mtk1 class.

Worker bundling: why three trampolines

new Worker(new URL("./relative", import.meta.url)) is the only shape webpack 5 treats as a worker entry point AND that esbuild can resolve at spec pre-bundle time. Inlining new URL("@codingame/...", import.meta.url) makes webpack happy but esbuild treats the spec as a literal relative URL and the spec pre-bundle fails. A fileReplacements-swapped factory file works but adds an angular.json delta. Three thin re-export trampolines satisfy both bundlers with zero angular.json change.

What gets dropped

  • @codingame/monaco-vscode-textmate-service-override — now a transitive of the v25 wrapper; not needed at top level
  • @codingame/monaco-vscode-theme-defaults-default-extension — same; pulled in transitively by the language extensions
  • @codingame/monaco-vscode-r-default-extension — upstream stopped publishing; R UDF editor retired
  • vscode (package alias + resolutions) — v10 no longer needs the alias for type-only vscode.Uri references
  • webpack-bundle-analyzer + analyze script — dead; script never ran in CI
  • content-disposition + @types/content-disposition — dead require, never read

Any related issues, documentation, discussions?

Closes #4996.

How was this PR tested?

  • yarn install resolves cleanly
  • yarn build exits 0; worker chunks identical hashes/sizes (~1.3 MB textmate, ~518 KB ext-host, ~220 KB editor)
  • yarn test stays at 63 / 269 spec parity

Manual smoke-test plan:

  • Open a workflow with a Python UDF — confirm syntax highlighting renders correctly on first open
  • Set / clear / step through a breakpoint with the debugger panel
  • Verify the AI-assistant "Add Type Annotation" / "Add All Type Annotations" actions still work
  • Check that two browser tabs editing the same workflow show each other's coeditor cursors
  • Run a Java UDF and confirm the editor shows Java syntax (no LSP, just grammar)

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.7)

Bumps monaco-languageclient 8.8.3 -> 10.7.0 and the @codingame/monaco-vscode-*
family 8.0.4 -> 25.1.2. Drops monaco-editor-wrapper entirely; the editor
component now drives the underlying triplet (MonacoVscodeApiWrapper +
LanguageClientWrapper + EditorApp) directly.

Notable changes:

* code-editor.component.ts rewritten against the v10 API. The vscode-api
  wrapper is started once per process; the default-extension activations
  await each extension's whenReady() so the TextMate grammars register
  before the editor opens, and a forceTokenization pass on first paint
  guarantees colours land before Monaco's initial render.
* Worker bootstraps live in src/.../workers/. The codingame-shipped worker
  files re-export internals via single-line imports that webpack only emits
  as static assets, so the trampolines force webpack to bundle the worker
  dep tree under the new Worker(new URL(...)) pattern.
* custom-webpack.config.js gains a oneOf rule that runs codingame CSS
  through css-loader with exportType: css-style-sheet, asset/resource for
  the bundled svg/ttf/png/woff, and an alias that redirects the css/style
  loader runtime back to its real install (codingame's deeper namespace
  breaks the relative paths css-loader generates). Patches a known
  empty-path crash in license-webpack-plugin.
* jsdom polyfill adds Constructable Stylesheets, matchMedia, CSS.escape,
  and registers a Node ESM loader hook (jsdom-css-loader-hook.mjs) that
  short-circuits .css imports to an empty module - needed because
  @angular/build:unit-test pre-bundles specs with externalPackages:true,
  so server.deps.inline never sees the codingame chain.
* All five previously-excluded specs (code-editor, codearea-custom-template,
  time-travel, versions-list, operator-property-edit-frame) run again.
* LICENSE-binary regenerated from dist/3rdpartylicenses.json (119 -> 123
  packages: more codingame sub-modules; vscode-textmate / vscode-oniguruma
  / monaco-editor-wrapper dropped; dompurify added).

Verified: yarn build, yarn test (63 files, 269 tests, parity with main),
yarn format:ci. Browser smoke test against a running backend confirmed
Python TextMate highlighting, editor mount, and worker spawn.
The package's npm tarball ships an older BSD-3-Clause LICENSE file but
the d3-path project's upstream LICENSE (https://github.com/d3/d3-path)
is ISC. license-webpack-plugin reported BSD-3-Clause off the tarball,
which is the historical artifact rather than the project's current
license. Move it back into the ISC bucket alongside the rest of the d3
ecosystem.
* download.service.ts had `var contentDisposition = require("content-disposition")`
  that was never read or called — removed the dead require, then dropped both
  `content-disposition` and `@types/content-disposition` from package.json.
* `webpack-bundle-analyzer` was only referenced from the `analyze` npm script
  and never invoked from CI. Dropped both the dep and the script.
The v10 commit listed every codingame service-override package texera
might touch, but six of them are pulled in transitively by
monaco-languageclient itself (or by another codingame override) and were
never imported from texera's source. Drop the redundant declarations:

  - configuration-service-override   (mlc dep)
  - editor-service-override          (mlc dep)
  - extensions-service-override      (mlc dep)
  - languages-service-override       (mlc dep)
  - theme-service-override           (mlc dep)
  - keybindings-service-override     (pulled in via views- and
                                      workbench-service-override)

Bundle output and LICENSE-binary are unchanged — only package.json gets
shorter. Verified with yarn build + yarn test (63 files, 269 tests).
The trampoline at `workers/textmate.worker.ts` deep-imports
`@codingame/monaco-vscode-textmate-service-override/worker`, but the
package is also a direct dependency of `monaco-languageclient@10.7.0`,
so the resolution still works without an explicit declaration. Drop the
dep and let mlc pin it.
…ension

`code-editor.component.ts` `import("@codingame/monaco-vscode-theme-defaults-default-extension")`
still resolves: the package is a direct dependency of
`monaco-languageclient@10.7.0`. Same pattern as the previous textmate
override drop — let mlc pin it.
Drops the R syntax-highlighting branch from the code editor + the
@codingame/monaco-vscode-r-default-extension dependency. The backend
RUDF / RUDFSource operators still exist; opening their code editor now
falls through to the default Java highlighting (degraded but functional).

* code-editor.component.ts: remove the R language detection branch and
  the .r file-suffix mapping; drop the dynamic-import of the R default
  extension from the bootstrap chain.
* code-debugger.component.ts: drop the side-effect import of the R
  default extension.
* package.json + LICENSE-binary: drop the codingame R extension.

Verified: yarn build, yarn test (63 / 269 — parity).
Both consumers — texera's own `dependencies.vscode` and
`monaco-languageclient@10.7.0`'s `dependencies.vscode` — already alias
to `npm:@codingame/monaco-vscode-extension-api`, so yarn unifies them
without a `resolutions` entry. Verified yarn.lock still resolves
`vscode@npm:` to the codingame extension api.

The `monaco-editor` resolution stays — it's load-bearing because
`monaco-breakpoints` declares `monaco-editor: ^0.39.0` (vanilla) and
without forcing the alias yarn would install two competing Monaco
copies.
No texera source file imports from `vscode`. The package was only there
to align with the codingame convention (`vscode` aliased to
`@codingame/monaco-vscode-extension-api`). `monaco-languageclient@10.7.0`
already declares the same alias as one of its dependencies, so the
package still installs transitively for any dep chain that needs it.
The codingame worker bundles were previously pulled in via three single-line
trampoline files in `code-editor-dialog/workers/`, each re-exporting one
upstream worker entry. Webpack 5 recognises `new Worker(new URL("@pkg/...",
import.meta.url))` natively and bundles the full dep tree into a worker
chunk, so the trampolines aren't needed for the production build.

The test pipeline blocks the truly-inline form: `@angular/build:unit-test`
runs `@angular/build:application` (esbuild) regardless of `customWebpackConfig`,
and esbuild resolves `new URL(spec, import.meta.url)` literally relative to
the source file rather than through node_modules. To keep tests buildable,
the URL refs live in a single sibling file that's swapped for a no-op stub
via `fileReplacements` on a new `gui:build-test` target.

  Before                                 After
  ────────────────────────────────────── ──────────────────────────────────────
  workers/editor.worker.ts               codingame-worker-factory.ts
  workers/extension-host.worker.ts       codingame-worker-factory.stub.ts
  workers/textmate.worker.ts
  (3 opaque trampolines)                 (1 readable factory + 1 test stub)

Worker chunks are byte-identical to the pre-refactor build (same content
hashes, same sizes: 1.3MB textmate / 518KB ext-host / 220KB editor).
Tests stay at 63 / 269 parity.
The Node ESM loader hook that strips transitive `.css` imports during specs
used to live in its own `src/jsdom-css-loader-hook.mjs` sidecar, registered
via `module.register(pathToFileURL(...))`. `module.register` accepts any
module URL though, including `data:` URLs — folding the hook source into
the registration call lets the sidecar file go away entirely.

The vitest.config.ts cross-reference comment that pointed at the hook is
also dropped since it duplicates what the polyfill file already documents.

Tests stay at 63 / 269 parity.
Tighten dead/redundant patches that crept in during the v10 migration:

* Drop the unused `getEnhancedMonacoEnvironment` import — its only consumer
  lives in `codingame-worker-factory.ts` now.
* Replace the 3-way `||` operator-type check with a `static readonly Set`
  + `.has(...)`; fold `generateLanguageTitle` into `setLanguage`.
* `ngOnDestroy`: drop `isDefined(monacoBinding)` (optional chaining) and
  `isDefined(workflowVersionStreamSubject)` (always assigned).
* `initializeMonacoEditor`: drop the redundant
  `switchMap(editor => of(editor ?? editorApp?.getEditor()))` —
  `startEditor()` already returns `editorApp.getEditor()` and `catchError`
  covers the timeout fallback. Remove the now-unused `switchMap` import.
* `acceptCurrentAnnotation`: drop the redundant inner
  `if (currentRange && currentSuggestion)` — the early-return guard above
  already proves both fields are truthy.
* `getCoeditorCursorStyles`: lift `clientId`/`color` to locals (each was
  repeated up to 3× in the same string).
* Remove the orphaned `// NOTE: dynamic imports ...` comment from the
  imports block; the function it references documents the rationale.

Net: -17 lines (650 -> 633). Build exits 0; tests stay at 63 / 269 parity.
Comparing against the TypeFox monaco-languageclient `verify/angular`
example surfaced two clean-ups:

* Drop the dynamic `import("@codingame/monaco-vscode-theme-defaults-default-extension")`
  — that package is no longer a declared dep (removed earlier in this branch);
  the dynamic import only resolved by transitive luck and would silently break
  if any sibling package stops pulling it in.

* Rewrite `ensureVscodeApiStarted` and the diff-editor `from(promise.then(...))`
  pipeline as plain `async`/`await`. The chained `.then().then().then()` shape
  was a holdover; the upstream example does the wrappers linearly.
  `apiWrapperStartPromise` now uses the `??=` idiom so the singleton-init
  is one expression instead of an `if` block.

Worker-factory note: upstream uses `monacoWorkerFactory: configureDefaultWorkerFactory`
which only works on Vite (it sets `getWorkerUrl`, depends on
`@codingame/esbuild-import-meta-url-plugin`). Our webpack pipeline still
needs `registerCodingameWorkers` to set `getWorker` directly so webpack-5
can statically bundle the worker chunks — verified the package paths it
uses match upstream's `defineDefaultWorkerLoaders` verbatim.

Net: -13 lines (633 -> 620). Build exits 0; tests stay at 63 / 269 parity.
…sting test config

Earlier in this branch the worker-factory swap lived on a brand-new
`gui:build-test` target backed by `@angular/build:application`. That was
overkill — `@angular-builders/custom-webpack:browser` already accepts
`fileReplacements` in its schema, and the build target already had a
`configurations.test` slot that the unit-test runner reads through
`gui:build:test`. Moved the replacement entry into that existing slot,
dropped the new build target, and reverted the test target's `buildTarget`
back to `gui:build:test`.

angular.json diff vs main shrinks from ~21 lines (new target + buildTarget
rename + orphan-block removal) to +6 lines (one fileReplacements array).

Build exits 0; tests stay at 63 / 269.
…ngular.json delta

Earlier the worker URL refs were inlined as `new URL("@codingame/...",
import.meta.url)` inside `codingame-worker-factory.ts`, with a
`fileReplacements` swap on `gui:build:test` redirecting to a no-op stub
during specs (esbuild can't resolve package-specifier URLs the way
webpack does). Going back to a relative-path `new URL("./workers/X",
import.meta.url)` pattern lets esbuild resolve the spec against the
filesystem and webpack still treats it as a worker entry.

  Net diff vs main
  ─────────────────────────────────────────────────────────────────────
  angular.json              0 lines  (was +6)
  tsconfig.app.json         +4 lines (the 3 trampolines listed in `files`)
  src/.../workers/*.ts      3 trampoline files (one-line side-effect imports)
  src/.../codingame-...     deleted (factory + stub)

So the worker plumbing now costs +1 source file (3 trampolines vs 2 factory
files) but `angular.json` becomes a no-op vs main and tests + build keep
the same parity (63 / 269 specs, byte-identical worker chunks).
Copilot AI review requested due to automatic review settings May 9, 2026 07:07
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI labels May 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Upgrades Texera’s frontend Monaco/VS Code language-client integration to monaco-languageclient v10 and the matching @codingame/monaco-vscode-* v25 stack, updating editor initialization, worker bundling, and build/test plumbing while cleaning up now-unneeded dependencies (including retiring the R UDF editor path).

Changes:

  • Rewrite the code editor integration to the v10 wrapper triplet (MonacoVscodeApiWrapper + EditorApp + LanguageClientWrapper) and add worker trampolines.
  • Update webpack + test environment to handle codingame v25 CSS/assets and jsdom gaps (Constructable Stylesheets, CSS imports under Node ESM, etc.).
  • Dependency/license list updates and removal of unused deps/scripts (e.g., content-disposition, webpack-bundle-analyzer, R extension).

Reviewed changes

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

Show a summary per file
File Description
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts Reworked Monaco/LSP startup for monaco-languageclient v10, worker wiring, tokenization workaround.
frontend/src/app/workspace/component/code-editor-dialog/workers/editor.worker.ts Trampoline worker entry for codingame editor worker.
frontend/src/app/workspace/component/code-editor-dialog/workers/extension-host.worker.ts Trampoline worker entry for codingame extension host worker.
frontend/src/app/workspace/component/code-editor-dialog/workers/textmate.worker.ts Trampoline worker entry for codingame textmate worker.
frontend/custom-webpack.config.js Webpack rules/aliases for codingame v25 assets/CSS + license-webpack-plugin workaround.
frontend/src/jsdom-svg-polyfill.ts Vitest/jsdom polyfills + Node ESM loader hook to swallow transitive .css imports.
frontend/vitest.config.ts Removes now-unneeded dependency inlining workaround for CSS imports.
frontend/src/tsconfig.app.json Adds worker trampoline files to compilation inputs.
frontend/src/app/workspace/component/code-editor-dialog/code-debugger.component.ts Removes R extension import to match retired R support.
frontend/src/app/dashboard/service/user/download/download.service.ts Drops unused content-disposition require.
frontend/package.json Bumps monaco-languageclient/codingame deps and removes unused packages/scripts.
frontend/LICENSE-binary Updates third-party license list for new dependency graph.
frontend/yarn.lock Lockfile updates reflecting dependency upgrades/removals.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts Outdated
Comment thread frontend/custom-webpack.config.js Outdated
`format:ci` flagged two backtick-delimited string literals with no
interpolation in `getCoeditorCursorStyles`. Switched them to double
quotes so the file matches prettier-eslint's expected output and the
CI gate passes.

No behaviour change.
CI's `check_binary_deps.py` flagged seven packages claimed by
`LICENSE-binary` that the production build no longer bundles. The bullets
are leftover from earlier dep cleanups in this branch — `r@1.0.0` came
from retiring R UDF support, `content-disposition` from dropping the
dead `require` in `download.service.ts`, and the rest
(`base64-js`, `buffer`, `ieee754`, `path-browserify`, `safe-buffer`)
are browserify shims that became unused once the codingame v25 stack
took over from v8.

`check_binary_deps.py --ignore-transitive-version npm frontend/dist/3rdpartylicenses.json`
now reports `OK: 113 npm packages match LICENSE-binary.`
@codecov-commenter

codecov-commenter commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.42466% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.75%. Comparing base (97ce29a) to head (5f15a30).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ponent/code-editor-dialog/code-editor.component.ts 53.42% 30 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4997      +/-   ##
============================================
- Coverage     59.80%   59.75%   -0.06%     
  Complexity     3290     3290              
============================================
  Files          1131     1131              
  Lines         43770    43805      +35     
  Branches       4745     4745              
============================================
- Hits          26178    26177       -1     
- Misses        16161    16198      +37     
+ Partials       1431     1430       -1     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 97ce29a
agent-service 44.59% <ø> (ø) Carriedforward from 97ce29a
amber 64.75% <ø> (ø) Carriedforward from 97ce29a
computing-unit-managing-service 0.00% <ø> (ø) Carriedforward from 97ce29a
config-service 52.30% <ø> (ø) Carriedforward from 97ce29a
file-service 62.81% <ø> (ø) Carriedforward from 97ce29a
frontend 51.42% <53.42%> (-0.09%) ⬇️
notebook-migration-service 78.57% <ø> (ø) Carriedforward from 97ce29a
pyamber 91.18% <ø> (ø) Carriedforward from 97ce29a
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 97ce29a

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…earing

Probed each block by removing it and re-running yarn build:ci /
check_binary_deps.py / yarn test. Dropped the parts that webpack 5
defaults or codingame v25 already cover, kept the parts that produce
real `Module parse failed` / license-check failures when removed.

Removed (verified dead via probe):
* license-webpack-plugin WebpackFileSystem monkey-patch (38 lines).
  `handleMissingLicenseText: () => null` already absorbs the ENOENT cases.
* `module.parser.javascript.url: true`. Webpack 5 enables this by default.
* `resolve.alias` for `@codingame/css-loader` -> `css-loader` (13 lines).
  codingame v25 no longer trips the path-resolution quirk that was
  emitting `node_modules/@codingame/css-loader/...` import specs.
* `excludedPackageTest: (name) => !name`. Subsumed by handleMissingLicenseText.

Kept (verified load-bearing):
* `asset/resource` rule for codingame `.svg/.ttf/.png/.woff*`. Without it
  webpack tries to parse the raw assets as JS and the build hits
  `Module parse failed: Unexpected token (1:0)` on the first .svg.
* CSS `oneOf`: codingame -> css-loader with `exportType: 'css-style-sheet'`,
  monaco-breakpoints -> style-loader + css-loader. Dropping either branch
  fails the build (codingame Constructable Stylesheets / breakpoints CSS).
* LicenseWebpackPlugin with custom `renderLicenses` -> bin/licensing/
  check_binary_deps.py reads the resulting JSON array shape.

Also pulled `path` to a top-level constant with a `nodeModule(...)` helper
(was repeating `require("path").resolve(__dirname, "node_modules/...")`)
and rewrote `renderLicenses` with optional chaining + a simpler sort.

Net: 154 -> 84 lines (-69).
yarn build:ci EXIT 0 with byte-identical worker chunks; check_binary_deps.py
reports `OK: 113 npm packages match LICENSE-binary`; yarn test stays at
63 / 269 parity.
Three Copilot review comments on apache#4997 — all valid, addressed in this
commit. (A fourth comment about an unused `const fs = require("fs")` in
custom-webpack.config.js was already resolved by de4d47b when the
license-webpack-plugin patch block was deleted as dead code.)

* Timeout scope: previously `LANGUAGE_SERVER_CONNECTION_TIMEOUT_MS`
  wrapped the entire `startEditor()` promise, including
  `ensureVscodeApiStarted()` and `EditorApp.start()`. On first load
  codingame v25 init can easily exceed 1s, so the timeout could fire
  before the editor mount completes and the rxjs `catchError` fallback
  would emit `undefined` (filtered out → MonacoBinding/actions/debugger
  never attach). Pulled the timeout out of the rxjs pipeline and into a
  `Promise.race` around `languageClientWrapper.start()` only; the editor
  mount is now always awaited to completion. Drops the unused `timeout`
  / `catchError` / `of` rxjs imports.

* Sticky rejection cache: `apiWrapperStartPromise` is shared across
  every CodeEditorComponent instance. If wrapper / extension activation
  ever rejected, the rejected promise would be returned forever and no
  subsequent open could retry. Wrapped the IIFE body in try/catch and
  reset the cache before re-throwing.

* Awareness-driven CSS injection: `getCoeditorCursorStyles` interpolates
  `coeditor.clientId` and `coeditor.color` (both writeable by any peer
  via yjs awareness) into a `<style>` tag passed through
  `bypassSecurityTrustHtml`. Anything that breaks out of the tag would
  land in the page as raw HTML. Added two tight allow-list regexes
  (digits-only id, hex / `rgb(a)` / `hsl(a)` colour) and bail out to an
  empty stylesheet if either fails.

Build + license + tests verified locally:
  yarn build:ci           EXIT 0   (worker chunks byte-identical)
  check_binary_deps.py    OK: 113 npm packages match
  yarn test               63 / 269 (parity)
Sweep over the files this PR touched looking for cleanups that didn't fit
into the focused commits:

* `code-debugger.component.ts`: drop the two static
  `import "@codingame/monaco-vscode-{python,java}-default-extension"` side-
  effect imports. They duplicate the dynamic imports inside
  `CodeEditorComponent.ensureVscodeApiStarted()` (which already loads them
  after `MonacoVscodeApiWrapper.start()` and awaits each `whenReady()`),
  and are documented elsewhere in this file as tree-shaken by the Angular
  build pipeline anyway, so the static form is dead either way.

* `jsdom-svg-polyfill.ts`:
  * The top-level docblock was talking about SVG geometry stubs, but the
    very next code in the file was the Node ESM `.css` loader hook —
    confusing for anyone landing in the file. Replaced the top docblock
    with a one-paragraph file overview and moved the SVG paragraph to
    where the SVG stubs actually are.
  * Merged the two `Document.prototype` lookups (`docProtoForCss` for
    `adoptedStyleSheets`, `docProto` for `queryCommandSupported`) into a
    single `docProto` block. Same constant, same prototype, two patches.
  * Tightened the `requestIdleCallback` block comment to a single
    leading-line comment to match the style of the SVG / loader-hook
    sections.

No behaviour change. yarn build:ci EXIT 0; check_binary_deps.py OK; yarn
test stays at 63 / 269.
@aglinxinyuan
aglinxinyuan requested a review from Yicong-Huang May 25, 2026 00:10
…t v10

The v10 upgrade replaced the single MonacoEditorLanguageClientWrapper (initAndStart) with a global MonacoVscodeApiWrapper + per-editor EditorApp, but code-editor.component.browser.spec.ts still swapped a fake `editorWrapper` and asserted on `wrapper.initAndStart(userConfig, host)` — so all 7 browser-mode tests failed with "called 0 times".

Drive the v10 seams instead: mock `monaco-languageclient/editorApp` to record the EditorAppConfig and start() host and hand back the fake editor, and stub the static `ensureVscodeApiStarted` so no real codingame worker / language extension loads. Assertions move off `userConfig.wrapperConfig.editorAppConfig.codeResources.main` to the EditorApp config's `codeResources.modified` (and `.original` on the diff path).
# Conflicts:
#	frontend/package.json
#	frontend/yarn.lock
Resolve frontend/package.json scripts conflict: keep main's build-version.js
prefix on build/build:ci, drop the analyze script (webpack-bundle-analyzer is
removed by this PR). yarn.lock reconciled (yarn install --immutable passes).
# Conflicts:
#	frontend/package.json
#	frontend/yarn.lock
yangzhang75 pushed a commit to yangzhang75/texera that referenced this pull request Jun 22, 2026
…otation-suggestion (apache#5004)

### What changes were proposed in this PR?

Three areas were under-tested in the frontend spec suite; this PR
addresses all three as one cohesive change:

- **`DownloadService`** — went from 8 active specs (all `it.skip(...)`)
to 14 active specs + 1 still-skipped (`createZip`, blocked by an
unrelated `import * as JSZip` interop bug). New coverage:
`downloadSingleFile` (incl. default-filename fallback +
`isLogin=false`), `downloadDataset`, `downloadDatasetVersion`,
`downloadWorkflow` (JSON shape), `downloadWorkflowsAsZip`, single-file
`downloadOperatorsResult`, the `No files to download` error path,
`getWorkflowResultDownloadability` HTTP wiring, plus matching
error-notification paths.
- **`CodeEditorComponent`** — went from 1 spec (`should create`) to 9
specs: operator-type → language detection for the three V2 Python
operator types and for plain Java / unknown types; the `languageTitle`
formula; `getCoeditorCursorStyles` for hex and rgba colours.
- **`AnnotationSuggestionComponent`** — new spec file (6 specs):
creation, default inputs, accept / decline event emission, independence
of the two emitters, `@Input` binding.

#### Why one test stays skipped

The multi-file `downloadOperatorsResult` path goes through `new
JSZip()`, where the production code does `import * as JSZip from
"jszip"`. The CI build flags this as `Constructing "JSZip" will crash at
run-time because it's an import namespace object`, and vitest reproduces
it as `__vite_ssr_import_* is not a constructor`. The spec is left as
`it.skip(...)` with a comment so it gets re-enabled when the source
switches to a default import.

### Any related issues, documentation, discussions?

Closes apache#5003.

### How was this PR tested?

Ran the suite on both branches under active development — confirms the
new specs are stable AND the in-flight v10 refactor on apache#4997 preserves
the externally-observable behaviour these specs exercise.

| Branch | Files | Tests |
|---|---|---|
| `main` | 66 passed / 2 skipped | 333 passed / 4 skipped / 2 todo |
| `chore/monaco-lsp-v10` (apache#4997) | 65 passed / 2 skipped | 297 passed /
4 skipped / 2 todo |

```
yarn test   # on main                     → 333 passed / 4 skipped / 2 todo
yarn test   # on chore/monaco-lsp-v10     → 297 passed / 4 skipped / 2 todo
```

The test-count delta between branches is unrelated PR drift (`main` has
picked up extra non-monaco specs that haven't been merged into the v10
branch yet); the spec files this PR adds run green on both.

### Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.7)
# Conflicts:
#	frontend/LICENSE-binary
#	frontend/package.json
#	frontend/yarn.lock
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang, @Ma77Ball, @PG1204
    You can notify them by mentioning @Yicong-Huang, @Ma77Ball, @PG1204 in a comment.

@Yicong-Huang

Copy link
Copy Markdown
Contributor

Not sure if it just happens to my local, but the editor is not functioning as expected

  • highlighting css is problematic, cannot see code clearly
  • command + / cannot comment/uncomment code.
  • python syntax highlighting is not working

See recording here:
https://github.com/user-attachments/assets/c6381d64-a545-4d45-937e-afaa6f0cd08b

@Yicong-Huang

Copy link
Copy Markdown
Contributor

Tracked down the broken syntax highlighting / cmd+/ / theme CSS from my comment above. Root cause is in the webpack build path.

Root cause

ng serve and yarn build go through webpack (@angular-builders/custom-webpack), where Angular's base config sets module.parser.javascript.url: false and worker: false (no webWorkerTsConfig in angular.json). With both off, webpack leaves new URL(..., import.meta.url) untransformed and inlines import.meta.url as a build-machine file:// path. From the actual dev-server compiled output:

fetch(new URL('../../../../../../../external/vscode-oniguruma/release/onig.wasm',
  "file:///Users/.../node_modules/@codingame/monaco-vscode-textmate-service-override/..."))

new Worker(new URL("./workers/editor.worker",
  "file:///Users/.../src/app/workspace/component/code-editor-dialog/code-editor.component.ts"))

In a real browser both fail: the onig.wasm fetch dies → the TextMate tokenizer never starts → no Python/Java highlighting; all three worker trampolines fail to spawn → language configuration and theming degrade (cmd+/ dead, unreadable token colors).

This PR removed the previous parser: { javascript: { url: true } } override in custom-webpack.config.js — it was there precisely for the wasm case (see angular-cli#24617) — and worker parsing has never been enabled in the webpack builds.

Why CI stayed green

The vitest specs (including the browser-mode ones) run through esbuild (@angular/build:unit-test), which natively supports both new URL(...) assets and new Worker(new URL(...)). Only the webpack-based ng serve / yarn build are affected — so every test passes while the served app is broken. The "worker chunks identical hashes/sizes" note in the PR description is worth re-checking: with parser.javascript.worker: false, webpack cannot have emitted those chunks.

Fix (verified against the dev-server output)

Restore/extend the parser override in frontend/custom-webpack.config.js:

module: {
  rules: [ /* unchanged */ ],
  parser: {
    javascript: {
      url: true,    // onig.wasm is fetched via new URL(..., import.meta.url)
      worker: true, // bundle the ./workers/* trampolines as worker chunks
    },
  },
},

With this applied, the compiled output contains zero file:// references; onig.wasm is emitted as a hashed asset (466 KB, served 200) and the three trampolines become real worker chunks (editor 384 KB, extension-host 4.4 MB, textmate 107 KB).

One gotcha for anyone re-testing: webpack's persistent cache (.angular/cache) does not track custom-webpack.config.js, so clear it before restarting ng serve — otherwise the stale broken output keeps being served.

Yicong-Huang and others added 2 commits July 4, 2026 15:03
Angular's webpack config disables new URL() asset modules (url: false)
and worker parsing (worker: false without webWorkerTsConfig), so
import.meta.url was inlined as a build-machine file:// path: the
textmate service's onig.wasm fetch and all three worker trampolines
failed at runtime, breaking syntax highlighting, comment toggling, and
token theming under ng serve / yarn build. Restore the url override
this PR dropped and enable worker parsing so the trampolines bundle
into real worker chunks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@aglinxinyuan

Copy link
Copy Markdown
Contributor Author

Thanks for the fix! I tested both the commit before the fix and the one after the fix, and both work on my end (Windows + Chrome). Since I can't reproduce the issue, I'll rely on your Safari test results and go ahead with the fix anyway, as it doesn't introduce any regressions from my side.

…CENSE-binary

The codingame v25 default-extension packages (java, python, theme-defaults)
are bundled under their inner package.json names/versions (java@1.0.0,
python@1.0.0, theme-defaults@1.0.0), which license-webpack-plugin emits into
3rdpartylicenses.json separately from the @codingame/...@25.1.2 wrappers.
Add the three MIT bullets so the LICENSE-binary drift check passes.

@Yicong-Huang Yicong-Huang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved with one more minor

Comment thread frontend/package.json
aglinxinyuan and others added 2 commits July 5, 2026 01:49
Revert the incidental removal of the "analyze" npm script and its
webpack-bundle-analyzer devDependency; both are unrelated to the
monaco-languageclient v10 upgrade and were flagged in review.
@aglinxinyuan
aglinxinyuan enabled auto-merge July 5, 2026 08:53
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 5, 2026
Merged via the queue into apache:main with commit 3d4301f Jul 5, 2026
19 checks passed
@aglinxinyuan
aglinxinyuan deleted the chore/monaco-lsp-v10 branch July 5, 2026 09:13
Yicong-Huang pushed a commit to Yicong-Huang/texera that referenced this pull request Jul 6, 2026
…yzer (apache#6156)

### What changes were proposed in this PR?

Remove the developer-only `analyze` npm script and its
`webpack-bundle-analyzer` devDependency from the frontend.

| File | Change |
| --- | --- |
| `frontend/package.json` | Drop the `"analyze"` script and the
`"webpack-bundle-analyzer": "4.5.0"` devDependency |
| `frontend/yarn.lock` | Regenerated — removes `webpack-bundle-analyzer`
and its **exclusive** transitive deps (`commander`, `duplexer`,
`gzip-size`, `mrmime`, `sirv`, `totalist`, `ws@^7.3.1`). Deps shared
with other packages (`acorn`, `lodash`, `opener`, `@polka/url`,
`acorn-walk`) stay — only their merged version-descriptors are trimmed |
| `frontend/README.md` | Drop the now-stale `Bundle analyzer → yarn
analyze` row from the "Common commands" table |

The `analyze` script — `ng build --configuration=production --stats-json
&& webpack-bundle-analyzer dist/stats.json` — is a manual, on-demand
bundle-inspection helper. It is not invoked by any build, CI, container,
or test path, and `webpack-bundle-analyzer` is not imported
programmatically anywhere in the repo, so both the script and its
devDependency are safe to drop.

### Any related issues, documentation, discussions?

Closes apache#6161.

Split out of apache#4997 per review feedback in
apache#4997 (comment), where
this removal was flagged as unrelated to the monaco-languageclient v10
upgrade. That PR restores the script; this PR removes it independently
so the change is reviewed on its own.

### How was this PR tested?

- **Lockfile consistency:** regenerated `frontend/yarn.lock` with `yarn
install --mode=update-lockfile` (Yarn 4.14.1). A second pass produced no
further changes, so CI's `yarn install --immutable` stays green.
- **Scope check:** confirmed the `yarn.lock` diff is limited to the
`webpack-bundle-analyzer` subtree — shared transitive deps remain; only
merged version-descriptors were trimmed (e.g. `acorn`'s `^8.0.4` spec
dropped, the rest kept).
- **No dangling references:** whole-repo grep for
`webpack-bundle-analyzer` / `BundleAnalyzerPlugin` (no source imports),
no custom webpack config under `frontend/`, and no hits in
`.github/workflows/**`, Dockerfiles, shell scripts, `nx.json`,
`angular.json`, or any `LICENSE-binary` (dev-only tool, so no
license-manifest entry to sync).
- `frontend/package.json` re-validated as parseable JSON after the edit.

### Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8 [1M context])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade frontend to monaco-languageclient v10

4 participants