chore(frontend): upgrade frontend to monaco-languageclient v10#4997
Open
aglinxinyuan wants to merge 37 commits into
Open
chore(frontend): upgrade frontend to monaco-languageclient v10#4997aglinxinyuan wants to merge 37 commits into
aglinxinyuan wants to merge 37 commits into
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 45.41% 45.40% -0.01%
Complexity 2222 2222
============================================
Files 1042 1042
Lines 39989 40024 +35
Branches 4260 4260
============================================
+ Hits 18160 18172 +12
- Misses 20716 20739 +23
Partials 1113 1113
*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.
aglinxinyuan
added a commit
that referenced
this pull request
May 12, 2026
…otation-suggestion (#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 #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 #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` (#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)
Contributor
Author
Added in PR #5004. |
# Conflicts: # frontend/package.json # frontend/yarn.lock
Two of the five new Copilot review comments on apache#4997 were real and are fixed here; the rest are false positives and get replies on the threads without code changes. * `process.on(...)` listener leak — vitest re-evaluates this setup file once per spec file, and each evaluation was attaching fresh `uncaughtException` / `unhandledRejection` handlers. After ~11 specs Node warns with `MaxListenersExceededWarning` and the benign-error filter reruns on every captured rejection. Gated the registration behind the same `Symbol.for(...)`-keyed `globalThis` flag pattern the CSS loader hook already uses. * CSS-import stub shape — the CSS ESM hook was resolving every `.css` import to `export default {}`. Any transitive consumer reading `.replaceSync` / `.cssRules` off the imported value (which the codingame v25 `css-style-sheet` export form does) would crash. The stub module now exports a `CSSStyleSheet`-shaped object with the no-op methods our `InertCSSStyleSheet` polyfill mirrors. yarn test now passes 71/73 files (442/446 specs); 2 specs stay skipped and 2 todos as before. yarn build:ci EXIT 0; license check OK: 113.
`check_binary_deps.py` on Ubuntu-CI reported `java@1.0.0`, `python@1.0.0`, and `theme-defaults@1.0.0` as STALE — i.e., claimed by `LICENSE-binary` but absent from `frontend/dist/3rdpartylicenses.json`. They came in via the merge from main and were never bundled by this branch's production build (the codingame default-extension dynamic imports are tree-shaken differently here). Removing the bullets to match what the build actually emits. (Local matches CI on the build step, so this commit just realigns `LICENSE-binary` with the dist that ships.)
Contributor
|
I manually canceled test after 1+ hr. |
Contributor
Author
Contributor
Author
|
@Yicong-Huang, can you review this PR? |
Yicong-Huang
requested changes
May 19, 2026
Contributor
Yicong-Huang
left a comment
Yicong-Huang
left a comment
There was a problem hiding this comment.
Please keep the changes to only upgrade related changes. I see you are doing a few other changes which are hardly to be related with the upgrade:
- remove support for R language/ R UDF. We do not ship the R UDF engine support in apache release. But let's keep the frontend code so that users can use R with plugin.
- custom web pack and polyfill file changes. I believe some changes are needed as we are changing the bundle packaging method due to new Monaco language client, but I think the changes are far more than needed.
- commands in package.json. I think they are not related.
Contributor
|
Also the test coverage is not enough.
I especially want to make sure the following can still work. If we do not currently have test coverage, it will be ideal to add tests for them first before upgrade.
|
Addresses @Yicong-Huang's review on apache#4997: R UDF is now distributed as a non-apache plugin, so the editor frontend still needs to open `RUDF` / `RUDFSource` operators in R mode even though Texera proper no longer ships the backend. * `code-editor.component.ts`: re-add a `R_OPERATOR_TYPES` set + the `else if (...)` branch in the constructor that maps `RUDFSource` / `RUDF` to `language = "r"`. Threaded `.r` through the two file-suffix call sites (`initializeMonacoEditor` and `initializeDiffEditor`) as a ternary chain. No language-client wiring for R — there's no off-the-shelf R LSP we ship; the editor still gives basic editing + the model URI carries the `.r` suffix so any future LSP would key off it. * `code-editor.component.spec.ts`: drop the "R UDF retired" preamble, add `R_OPERATOR_TYPES = ["RUDFSource", "RUDF"]` alongside the Python set, augment the stub-metadata service with synthetic R schemas, and add a `R_OPERATOR_TYPES.forEach(...)` test block that pins `language === "r"` and `languageTitle === "R UDF"` for each type. * `jsdom-svg-polyfill.ts`: tighten the top-of-file comment (it was ~14 lines for a 4-line concept) and call out explicitly that most blocks are codingame-v25-driven — the jointjs SVG geometry stubs are the only block that predates this PR's monaco-languageclient v10 upgrade. yarn test now passes 71/73 files (444/446 specs incl. the 2 new R tests); yarn build:ci EXIT 0; license check OK: 113.
Contributor
Author
I tried to improve the general test coverage for the editor in PR #5004. I also tested the editor features manually for this PR. If full test coverage is required for all 3 points you mentioned before upgrading, I don't plan to move forward with this PR, as I don't have enough bandwidth. |
Contributor
Author
|
Not planned. |
Contributor
|
Let's fill the tests separately before upgrading. Thanks for the effort! |
Contributor
Author
|
Test cases added in PR #5191 by @Yicong-Huang |
Conflicts in `frontend/package.json` and `frontend/yarn.lock` from two changes that landed on main after the last rebase: * `@lezer/python@1.1.18` was added in apache#5043 for the new Python UDF UI parameter form. Kept as-is — independent of the monaco upgrade. * `@codingame/monaco-vscode-{java,python}-default-extension` jumped from `8.0.4` (main) to `25.1.2` (PR side). Kept the PR side; that's the whole point of this branch. `r-default-extension` stays dropped per the PR's `What gets dropped` section. Knock-on: apache#5191 added unit tests on main that pin a `getFileSuffixByLanguage` private method and stub `editorWrapper` in the `acceptCurrentAnnotation` path. The v10 rewrite had inlined the suffix logic as a ternary at the two call sites and renamed `editorWrapper` -> `editorApp`. Restored the helper (cleaner anyway, and brings back `javascript` / case-insensitivity that the ternary silently dropped) and pointed the test stub at `editorApp` so both PRs' test surfaces stay green together. Verified on the merged tree: * `yarn install` clean (only the pre-existing monaco-breakpoints peer-dep warning, same as before the merge) * `yarn build` exits 0 * `yarn test --watch=false` -> 510 pass / 0 fail / 2 skip / 2 todo * `yarn lint` and `yarn format:ci` both clean
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)