Copilot commented on code in PR #3309:
URL: https://github.com/apache/apisix-dashboard/pull/3309#discussion_r2872357797
##########
src/components/form/Editor.tsx:
##########
@@ -145,9 +145,7 @@ export const FormItemEditor = <T extends FieldValues>(
trigger(props.name);
}}
onMount={(editor) => {
- if (process.env.NODE_ENV === 'test') {
- window.__monacoEditor__ = editor;
- }
+ window.__monacoEditor__ = editor;
}}
Review Comment:
`window.__monacoEditor__` is now assigned unconditionally. Since this
creates a long-lived global reference, it can keep disposed editors/models from
being garbage-collected and can leave a stale instance around after unmount.
Consider either (a) clearing the global on unmount (if `@monaco-editor/react`'s
`onUnmount` is available) or (b) guarding the assignment behind an explicit
E2E/dev flag so production doesn’t retain this global reference.
```suggestion
(window as any).__monacoEditor__ = editor;
}}
onUnmount={(editor) => {
const w = window as any;
if (w.__monacoEditor__ === editor) {
w.__monacoEditor__ = undefined;
}
}}
```
##########
e2e/utils/ui/index.ts:
##########
@@ -64,11 +64,24 @@ export async function uiFillHTTPStatuses(
}
}
-export const uiClearMonacoEditor = async (page: Page) => {
- await page.evaluate(() => {
- const editor = window.__monacoEditor__;
- editor.getModel()?.setValue('');
- });
+export const uiClearMonacoEditor = async (page: Page, editorLoc?: Locator) => {
+ const isSet = await page.evaluate(() => window.__monacoEditor__ !==
undefined).catch(() => false);
+ if (isSet) {
+ await page.evaluate(() => {
+ const editor = window.__monacoEditor__;
+ editor?.getModel()?.setValue('');
+ });
Review Comment:
`uiClearMonacoEditor` currently performs two separate `page.evaluate` calls
(one to check presence, one to clear). This can be simplified into a single
evaluate (attempt to clear if present and return a success flag), which reduces
round-trips and makes it easier to fall back to the keyboard-based clearing
when the evaluate fails.
```suggestion
const clearedViaMonaco = await page
.evaluate(() => {
try {
const editor = (window as any).__monacoEditor__;
if (!editor || typeof editor.getModel !== 'function') {
return false;
}
const model = editor.getModel();
if (!model || typeof model.setValue !== 'function') {
return false;
}
model.setValue('');
return true;
} catch {
return false;
}
})
.catch(() => false);
if (clearedViaMonaco) {
```
--
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]
