[WIP] Add a Vite-based manual test server#1374
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| panel.classList.toggle( 'shell-instructions--open', isOpen ); | ||
| panel.setAttribute( 'aria-hidden', String( !isOpen ) ); | ||
| panel.toggleAttribute( 'inert', !isOpen ); | ||
| trigger!.setAttribute( 'aria-expanded', String( isOpen ) ); |
There was a problem hiding this comment.
Missing null guard for trigger in toggleInstructions
Medium Severity
The toggleInstructions function checks if panel is null and returns early, but uses trigger! without any null guard. If panel is found in the DOM but trigger isn't (e.g., due to unexpected DOM mutations by a manual test script), trigger!.setAttribute(...) throws a runtime error. The null check pattern is inconsistent — panel is properly guarded, but trigger relies on a non-null assertion that silences the type checker while leaving a potential crash path.
Reviewed by Cursor Bugbot for commit 638bdb3. Configure here.
|
|
||
| next(); | ||
| } ); | ||
| }, |
There was a problem hiding this comment.
Duplicated middleware setup across server hooks
Low Severity
The configureServer and configurePreviewServer hooks contain byte-for-byte identical middleware registration code. Extracting the shared setup into a helper function would reduce duplication and ensure future changes to middleware ordering or logic are applied consistently to both the dev and preview servers.
Reviewed by Cursor Bugbot for commit 638bdb3. Configure here.
| const MANUAL_SHELL_TEMPLATE_FILE_PATH = path.resolve( MANUAL_THEME_ROOT, 'shell.html' ); | ||
| const MANUAL_SHELL_SCRIPT_FILE_PATH = path.resolve( MANUAL_THEME_ROOT, 'shell.ts' ); | ||
| const MANUAL_CATALOG_PUBLIC_PATH = toPublicFilePath( MANUAL_CATALOG_FILE_PATH, WORKSPACE_ROOT ); | ||
| const MANUAL_SHELL_SCRIPT_PUBLIC_PATH = toPublicFilePath( MANUAL_SHELL_SCRIPT_FILE_PATH, WORKSPACE_ROOT ); |
There was a problem hiding this comment.
Module-level constants capture process.cwd() at import time
Low Severity
WORKSPACE_ROOT and all derived constants (MANUAL_CATALOG_PUBLIC_PATH, MANUAL_SHELL_SCRIPT_PUBLIC_PATH) are computed as module-level constants using process.cwd() at import time, rather than inside the createManualTestsPlugin() factory function. This means the workspace root is frozen the moment the module is first imported, making the plugin impossible to test with different working directories and fragile if the module is imported before process.cwd() reflects the intended project root.
Reviewed by Cursor Bugbot for commit 47862c8. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e54c259. Configure here.
|
|
||
| import type { HotPayload, Plugin } from 'vite'; | ||
|
|
||
| const MANUAL_REFRESH_EVENT_NAME = 'ckeditor5-manual:refresh-available'; |
There was a problem hiding this comment.
Duplicated event name constant across server and client
Low Severity
The MANUAL_REFRESH_EVENT_NAME string 'ckeditor5-manual:refresh-available' is independently defined in both the server-side plugin (src/manual-refresh-plugin/plugin.ts) and the client-side shell (theme/shell.ts). If one is updated without the other, the manual refresh feature silently breaks with no error. Since theme/shell.ts already imports types from src/, a lightweight shared constants module could eliminate this duplication.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e54c259. Configure here.
1 participant
🚀 Summary
Add a Vite-based manual test server.
📌 Related issues
💡 Additional information
Still WIP.