chore(frontend): upgrade frontend to monaco-languageclient v10#4997
Open
aglinxinyuan wants to merge 25 commits intoapache:mainfrom
Open
chore(frontend): upgrade frontend to monaco-languageclient v10#4997aglinxinyuan wants to merge 25 commits intoapache:mainfrom
aglinxinyuan wants to merge 25 commits intoapache:mainfrom
Conversation
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).
github-actions
Bot
added
dependencies
Contributor
There was a problem hiding this comment.
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.
`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 Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4997 +/- ##
=========================================
Coverage 42.72% 42.73%
Complexity 2186 2186
=========================================
Files 1031 1031
Lines 38152 38179 +27
Branches 4004 4002 -2
=========================================
+ Hits 16302 16317 +15
- Misses 20831 20846 +15
+ Partials 1019 1016 -3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…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.
Comment on lines
21
to
25
| import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; | ||
| import { SafeStyle } from "@angular/platform-browser"; | ||
| import "@codingame/monaco-vscode-python-default-extension"; | ||
| import "@codingame/monaco-vscode-r-default-extension"; | ||
| import "@codingame/monaco-vscode-java-default-extension"; | ||
| import { isDefined } from "../../../common/util/predicate"; | ||
| import * as monaco from "monaco-editor"; | ||
| import { MonacoBreakpoint } from "monaco-breakpoints"; |
Contributor
Author
There was a problem hiding this comment.
Already done — see e8e67b5. Both import "@codingame/monaco-vscode-{python,java}-default-extension" lines were dropped from code-debugger.component.ts, and the centralized ensureVscodeApiStarted() in code-editor.component.ts (where the editor stack is actually instantiated) is the only place loading them now. The static side-effect imports on the debugger were leftover duplicates and are gone.
Comment on lines
63
to
64
| // run under Vitest browser mode rather than jsdom (tracked in #4861). | ||
| function fakeMatrix() { |
Contributor
Author
There was a problem hiding this comment.
Good catch — fixed in 33eea83.
Vitest re-evaluates setupFiles once per spec file, so the unconditional module.register(...) was chaining a fresh .css ESM hook for every spec. Guarded the registration behind a Symbol.for("texera.cssLoaderHookRegistered") flag on globalThis so the hook is installed exactly once per process; subsequent setup invocations short-circuit before calling register.
Another sweep for things that didn't quite pull their weight:
* `code-editor.component.ts`:
* Drop the `apiWrapperStarted` static boolean; `apiWrapperStartPromise`
is now the single source of truth — once it resolves, every later
`await` returns immediately, and the failure branch already clears
it back to `undefined` so a retry runs the full start sequence
again. The boolean was just a sync fast-path masking the same
semantic.
* Simplify `getFileSuffixByLanguage`: the `javascript` case was
unreachable (the constructor only ever calls
`setLanguage("python" | "java")`, and R UDF support was retired
earlier in this branch). Collapse to a one-line ternary with a
`.py` default.
* `editor.worker.ts`: trim the canonical-rationale comment from 7 lines
to 3. The full reasoning is already documented in the
`monacoWorkerFactory` block in code-editor.component.ts; the worker
file only needs a pointer.
* `jsdom-svg-polyfill.ts`: convert the remaining `/** ... */` block
comments (CSSStyleSheet, CSS namespace, matchMedia, WebSocket,
ngZorro icon suppression) to leading `//` comments to match the style
of the SVG / loader-hook / requestIdleCallback sections that already
use them. The mixed style was visually noisy in a long polyfill file.
Same content, just one comment shape throughout.
No behaviour change.
yarn build:ci EXIT 0; check_binary_deps.py OK: 113; yarn test 63 / 269.
Aggressive line-count pass on the two heaviest PR-touched files.
* `jsdom-svg-polyfill.ts`:
* Add a single `G = globalThis as Record<string, any>` accessor and
use it everywhere instead of repeating
`(globalThis as unknown as { Foo?: { prototype: ... } }).Foo`
five times. Each cast site shrinks from ~3 lines to one identifier.
* Add `installIfMissing(proto, fns)` helper that takes a prototype +
map of `name -> impl` and only patches the missing slots. Replaces
six hand-rolled `if (typeof proto.X !== "function") proto.X = ...`
branches across the SVG / CSSStyleSheet / Document / CSS sections.
* Compact `fakeMatrix` / `fakeTransform` to drive their no-op stubs
from a `for-of` over the method-name list rather than a fan-out of
11 + 6 individual assignments.
* Use `??=` for the `CSS` namespace + `requestIdleCallback` /
`cancelIdleCallback` initialisation; collapse the matchMediaStub
object literal onto fewer lines.
* `code-editor.component.ts`: trim the 14-line `monacoWorkerFactory`
comment to 7 lines — same content, less prose. The component file is
the canonical home of the trampoline rationale; the worker files
point back at it.
Net for this commit: 2 files, +87 / -134 (-47). Cumulative for the
PR's two cleanup commits: +163 / -278 (-115). No behaviour change.
yarn build:ci EXIT 0 with byte-identical worker chunks; check_binary_deps.py
OK: 113; yarn test stays at 63 / 269.
…dies
* `code-editor.component.ts`:
* Inline the `setLanguage()` method into the constructor — it had
exactly one call site there. Saves the wrapper.
* Inline `getFileSuffixByLanguage()` at its two call sites as a
one-line ternary. With only `python` / `java` reachable post-R-UDF,
the helper was longer than the inline form.
* `jsdom-svg-polyfill.ts`:
* Pull the `fakeMatrix` / `fakeTransform` method-name lists into
space-separated string constants (`MATRIX_METHODS` /
`TRANSFORM_METHODS`) and `.split(" ")` in the loop. Prettier was
expanding the array literals to one item per line; the string form
fits on one line each.
* Convert the no-op method bodies in `InertCSSStyleSheet` and
`InertWebSocket` from block-form (`replace(): Promise<void> {
return Promise.resolve(); }`) to arrow-function fields (`replace =
() => Promise.resolve()`). Identical semantics for these stubs;
prettier keeps the arrow form on one line.
Net: 2 files, +24 / -50 (-26).
yarn build:ci EXIT 0; check_binary_deps.py OK: 113; yarn test 63 / 269.
… construction Two new Copilot review comments on apache#4997, plus a small drive-by: * `jsdom-svg-polyfill.ts`: vitest re-evaluates `setupFiles` once per spec file, so the unconditional `module.register(...)` was chaining a fresh `.css` ESM hook for every spec — slower (and noisier) module resolution across the run. Guard the registration with a `Symbol.for(...)`-keyed flag on `globalThis` so the hook installs exactly once per process. * `code-editor.component.ts` (Copilot — code-debugger static imports): already addressed in e8e67b5; reply on the thread. * `code-editor.component.ts` drive-by: `acceptCurrentAnnotation` was building a `monaco.Selection` from `currentRange` only to pass it to `insertTypeAnnotations`, which in turn only read `endLineNumber` / `endColumn` — fields `monaco.Range` already exposes. Dropped the `Selection` construction; `insertTypeAnnotations` now takes the `Range` directly. Switched the offset fallback to `??` so a real `0` offset isn't masked. yarn build:ci EXIT 0; check_binary_deps.py OK: 113; yarn test 63 / 269.
Contributor
|
do we want to add tests before we upgrade? just want to make sure things are not broken. |
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.
What changes were proposed in this PR?
Brings the frontend's Monaco / VS Code language-client stack up to
monaco-languageclientv10 (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
The editor component is rewritten against the new shape, plus a
whenReady()handshake on the default language extensions and a post-mountforceTokenizationloop to fix a TextMate first-paint race that otherwise renders every token as the defaultmtk1class.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. Inliningnew URL("@codingame/...", import.meta.url)makes webpack happy but esbuild treats the spec as a literal relative URL and the spec pre-bundle fails. AfileReplacements-swapped factory file works but adds anangular.jsondelta. Three thin re-export trampolines satisfy both bundlers with zeroangular.jsonchange.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 retiredvscode(package alias +resolutions) — v10 no longer needs the alias for type-onlyvscode.Urireferenceswebpack-bundle-analyzer+analyzescript — dead; script never ran in CIcontent-disposition+@types/content-disposition— deadrequire, never readAny related issues, documentation, discussions?
Closes #4996.
How was this PR tested?
yarn installresolves cleanlyyarn buildexits 0; worker chunks identical hashes/sizes (~1.3 MB textmate, ~518 KB ext-host, ~220 KB editor)yarn teststays at 63 / 269 spec parityManual smoke-test plan:
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.7)