Copilot commented on code in PR #5241: URL: https://github.com/apache/texera/pull/5241#discussion_r3308522774
########## frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts: ########## @@ -0,0 +1,323 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Browser-mode companion to code-editor.component.spec.ts. The sibling jsdom +// spec covers the constructor, language detection, getFileSuffixByLanguage, +// onFocus, getCoeditorCursorStyles, and the accept/reject annotation paths, +// but cannot reach anything gated on a real Monaco editor — the +// `initAndStart` subscribe body, `initializeDiffEditor`, AI-action run +// callbacks, `handleTypeAnnotation`'s position branch, and the resize +// handler. This spec drives those by swapping the component's editorWrapper +// for a fake-with-real-DOM and running in vitest's Playwright/Chromium +// browser mode, where monaco-editor's codingame fork can be imported without +// jsdom's missing-canvas / Node-Buffer-allocation tripwires (see the +// nodePolyfills entry in vitest.browser.config.ts). Review Comment: This comment points readers to a `nodePolyfills` entry in `vitest.browser.config.ts`, but the config added here uses a setup file plus `optimizeDeps` and has no `nodePolyfills` entry. Please update the reference so future maintainers can find the actual polyfill wiring. ########## frontend/src/browser-buffer-polyfill.ts: ########## @@ -0,0 +1,44 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// monaco-editor-wrapper + monaco-vscode-files-service-override dereference +// Node's Buffer (including Buffer.allocUnsafe) at module-evaluation time. +// Under jsdom-mode Vitest, the test runs on Node and Buffer is built-in. In +// browser-mode (Playwright/Chromium), Buffer doesn't exist, so importing +// monaco-editor-wrapper crashes at load. The `buffer` npm package is the +// browser polyfill of Node's API; expose its `Buffer` (and a minimal +// `process` shim) on the global so monaco's internals find them at the same +// names they'd find Node's. Has to run before any monaco import is +// evaluated, which is why this is wired into vitest.browser.config.ts as the +// FIRST setupFile — setupFiles run in order, before any test module loads. +// +// We use a namespace import (rather than a named or default import) because +// Vite's dep-optimizer rewrites `buffer@5`'s CJS exports in a way that +// exposes neither a `Buffer` named export nor a `default` export at +// module-eval time. The namespace object always has the optimizer-injected +// shape, so we read `Buffer` off it dynamically. +import * as bufferModule from "buffer"; Review Comment: This setup file imports the `buffer` package directly, but `frontend/package.json` does not declare `buffer` in dependencies or devDependencies. Because this relies on a transitively hoisted package, a dependency tree change can make the browser test target fail to resolve this import; add `buffer` as an explicit devDependency (and update the lockfile) before using it here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
