eslint - disallow querySelector and friends#274582
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new ESLint rule to discourage the use of DOM query methods (querySelector, querySelectorAll, getElementById, etc.) in the browser/electron-browser layer, promoting the use of dom.ts h() for building and accessing elements directly. The rule includes a comprehensive ignore list of existing files that use these methods. Additionally, ESLint disable comments are added to auxiliaryWindowService.ts to suppress warnings where querySelector is legitimately used to clone elements from the main window document to auxiliary windows.
- Adds
no-restricted-syntaxESLint rule for browser/electron-browser layer files to restrict DOM query methods - Includes 98 existing files in the ignore list that currently use these methods
- Adds inline ESLint disable comments in
auxiliaryWindowService.tsfor legitimate querySelector usage
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| eslint.config.js | Adds new ESLint rule configuration restricting DOM query methods with extensive ignore list for existing violations |
| src/vs/workbench/services/auxiliaryWindow/browser/auxiliaryWindowService.ts | Adds inline eslint-disable-next-line comments to suppress warnings for legitimate querySelector usage when cloning DOM elements |
| // eslint-disable-next-line no-restricted-syntax | ||
| const metaElement = mainWindow.document.querySelector(metaTag); |
There was a problem hiding this comment.
The eslint-disable comment lacks an explanation for why querySelector is necessary here. Add a brief inline comment explaining that querySelector is required because these are existing DOM elements from the main window that need to be discovered and cloned, not new elements being created.
| } | ||
| } | ||
|
|
||
| // eslint-disable-next-line no-restricted-syntax |
There was a problem hiding this comment.
The eslint-disable comment lacks an explanation for why querySelector is necessary here. Add a brief inline comment explaining that querySelector is required because this queries an existing icon link tag from the main window that needs to be cloned to the auxiliary window.
| // eslint-disable-next-line no-restricted-syntax | |
| // eslint-disable-next-line no-restricted-syntax -- querySelector is required here because this queries an existing icon link tag from the main window that needs to be cloned to the auxiliary window. |
No description provided.