0

Revert "WebUI: Enable @typescript-eslint/require-await check in build_webui()."

This reverts commit 32867d045f.

Reason for revert: Breaks the build.

FAILED: gen/ui/webui/resources/cr_components/cr_shortcut_input/eslint.config.mjs 
python3 ../../ui/webui/resources/tools/eslint_ts.py --in_folder gen/ui/webui/resources/cr_components/cr_shortcut_input/preprocessed --out_folder gen/ui/webui/resources/cr_components/cr_shortcut_input --config_base ../../../../../../../../ui/webui/resources/tools/eslint_ts.config_base.mjs --tsconfig tsconfig_build_ts.json --in_files cr_shortcut_input.ts cr_shortcut_input.html.ts cr_shortcut_util.ts
Traceback (most recent call last):
  File "/usr/local/google/home/sesse/chromium/src/out/Default/../../ui/webui/resources/tools/eslint_ts.py", line 78, in <module>
    main(sys.argv[1:])
  File "/usr/local/google/home/sesse/chromium/src/out/Default/../../ui/webui/resources/tools/eslint_ts.py", line 67, in main
    node.RunNode([
  File "/usr/local/google/home/sesse/chromium/src/third_party/node/node.py", line 38, in RunNode
    raise RuntimeError('Command \'%s\' failed\n%s' % (' '.join(cmd), err))
RuntimeError: Command '/usr/local/google/home/sesse/chromium/src/third_party/node/linux/node-linux-x64/bin/node /usr/local/google/home/sesse/chromium/src/third_party/node/node_modules/eslint/bin/eslint --color --quiet --config gen/ui/webui/resources/cr_components/cr_shortcut_input/eslint.config.mjs gen/ui/webui/resources/cr_components/cr_shortcut_input/preprocessed/cr_shortcut_input.ts gen/ui/webui/resources/cr_components/cr_shortcut_input/preprocessed/cr_shortcut_input.html.ts gen/ui/webui/resources/cr_components/cr_shortcut_input/preprocessed/cr_shortcut_util.ts' failed

Oops! Something went wrong! :(

ESLint: 9.10.0

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:371:5)
    at validateString (node:internal/validators:119:11)
    at Object.join (node:path:1172:7)
    at file:///usr/local/google/home/sesse/chromium/src/out/Default/gen/ui/webui/resources/cr_components/cr_shortcut_input/eslint.config.mjs?mtime=1739278158700:10:26
    at ModuleJob.run (node:internal/modules/esm/module_job:185:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:281:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at async loadFlatConfigFile (/usr/local/google/home/sesse/chromium/src/third_party/node/node_modules/eslint/lib/eslint/eslint.js:390:21)
    at async calculateConfigArray (/usr/local/google/home/sesse/chromium/src/third_party/node/node_modules/eslint/lib/eslint/eslint.js:473:28)

Original change's description:
> WebUI: Enable @typescript-eslint/require-await check in build_webui().
>
>  - Enabling `enable_type_aware_eslint_checks` by default in build_webui().
>  - Fixing a few remaining violations that were not already fixed in
>    precursor CLs.
>  - Fix the dependnecies of the ":lint" target for the case of
>    `use_javascript_coverage=true`
>  - Add a new "ESLint checks" section in styleguide/web/web.md explaining
>    the two different types of ESLint checks.
>  - Update Chromium styleguide additions to include what will be
>    enfroced by @typescript-eslint/require-await as part of this CL,
>    and @typescript-eslint/no-unnecessary-type-assertion in upcoming
>    CLs.
>
> Bug: 394634491
> Change-Id: I59a07d4e76339449c3e5ff79608b57154a3e32cb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6250398
> Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1418375}

Bug: 394634491
Change-Id: Iccfa9b2c94f608e193f95a7fe454abbec3c3f3b4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6253018
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Auto-Submit: Steinar H Gunderson <sesse@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Owners-Override: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1418592}
This commit is contained in:
Steinar H Gunderson
2025-02-11 05:49:25 -08:00
committed by Chromium LUCI CQ
parent 49d8d714a8
commit 9832bca736
8 changed files with 25 additions and 85 deletions
chrome/browser/resources/pdf/ink
components/webui/flags/resources
content/browser/resources
docs/webui
styleguide/web
ui/webui/resources
cr_components
certificate_manager
tools

@@ -36,7 +36,7 @@ export class InkApi {
return this.canvas_.getPDFDestructive(); return this.canvas_.getPDFDestructive();
} }
setCamera(camera: drawings.Box) { async setCamera(camera: drawings.Box) {
this.camera_ = camera; this.camera_ = camera;
this.canvas_.setCamera(camera); this.canvas_.setCamera(camera);
// Wait for the next task to avoid a race where Ink drops the camera value // Wait for the next task to avoid a race where Ink drops the camera value

@@ -400,7 +400,6 @@ export class FlagsAppElement extends CrLitElement {
* don't actually exist until after the template code runs; normal navigation * don't actually exist until after the template code runs; normal navigation
* therefore doesn't work. * therefore doesn't work.
*/ */
// eslint-disable-next-line @typescript-eslint/require-await
private async highlightReferencedFlag() { private async highlightReferencedFlag() {
if (!window.location.hash) { if (!window.location.hash) {
return; return;

@@ -267,7 +267,7 @@ async function renderUsageAndQuotaStats() {
} }
} }
function renderSimulateStoragePressureButton() { async function renderSimulateStoragePressureButton() {
getProxy().isSimulateStoragePressureAvailable().then(result => { getProxy().isSimulateStoragePressureAvailable().then(result => {
if (!result.available) { if (!result.available) {
document.body document.body

@@ -19,7 +19,7 @@ import type {XrFrameStatistics, XrLogMessage} from './xr_session.mojom-webui.js'
let browserProxy: BrowserProxy; let browserProxy: BrowserProxy;
function bootstrap() { async function bootstrap() {
browserProxy = BrowserProxy.getInstance(); browserProxy = BrowserProxy.getInstance();
assert(browserProxy); assert(browserProxy);
@@ -30,7 +30,7 @@ function bootstrap() {
renderSessionStatisticsContent(); renderSessionStatisticsContent();
} }
function setupSidebarButtonListeners() { async function setupSidebarButtonListeners() {
const deviceInfoButton = getRequiredElement('device-info-button'); const deviceInfoButton = getRequiredElement('device-info-button');
const sessionInfoButton = getRequiredElement('session-info-button'); const sessionInfoButton = getRequiredElement('session-info-button');
const runtimeInfoButton = getRequiredElement('runtime-info-button'); const runtimeInfoButton = getRequiredElement('runtime-info-button');
@@ -81,7 +81,7 @@ async function renderDeviceInfoContent() {
deviceInfoContent.appendChild(table); deviceInfoContent.appendChild(table);
} }
function renderSessionInfoContent() { async function renderSessionInfoContent() {
const sessionInfoContent = getRequiredElement('session-info-content'); const sessionInfoContent = getRequiredElement('session-info-content');
assert(sessionInfoContent); assert(sessionInfoContent);
@@ -116,7 +116,7 @@ async function renderActiveRuntimesTable(
runtimeInfoTable.recreateActiveRuntimesTable(activeRuntimes); runtimeInfoTable.recreateActiveRuntimesTable(activeRuntimes);
} }
function renderRuntimeInfoContent() { async function renderRuntimeInfoContent() {
const runtimeInfoContent = getRequiredElement('runtime-info-content'); const runtimeInfoContent = getRequiredElement('runtime-info-content');
assert(runtimeInfoContent); assert(runtimeInfoContent);
@@ -143,7 +143,7 @@ function renderRuntimeInfoContent() {
runtimeInfoContent.appendChild(runtimeChangelogTable); runtimeInfoContent.appendChild(runtimeChangelogTable);
} }
function renderSessionStatisticsContent() { async function renderSessionStatisticsContent() {
const sessionStatisticsContent = const sessionStatisticsContent =
getRequiredElement('session-statistics-content'); getRequiredElement('session-statistics-content');
assert(sessionStatisticsContent); assert(sessionStatisticsContent);

@@ -666,8 +666,8 @@ enable_source_maps: Defaults to "false". Incompatible with |optimize=true|.
Setting it to "true" turns on source map generation for a Setting it to "true" turns on source map generation for a
few underlying targets. See ts_library()'s few underlying targets. See ts_library()'s
|enable_source_maps| for more details. |enable_source_maps| for more details.
enable_type_aware_eslint_checks: Defaults to "true". Turns on additional enable_type_aware_eslint_checks: Defaults to "false". Setting it to "true" turns
type-aware ESLint checks. See on additional type-aware ESLint checks. See
eslint_ts() for more details. eslint_ts() for more details.
``` ```

@@ -302,25 +302,21 @@ JavaScript, but it is expected that all code should migrate to TS eventually.
### Style ### Style
The Chromium styleguide is combination of 3 components: See the [Google TypeScript Style
Guide](https://google.github.io/styleguide/tsguide.html) as well as
1. The [Google TypeScript Style Guide]( [ECMAScript Features in Chromium](es.md).
https://google.github.io/styleguide/tsguide.html) which is used as the
starting point.
2. [ECMAScript Features in Chromium](es.md), which specifies which ES features
should or shouldn't be used in Chromium.
3. Additional guidelines applied on top of #1, which are listed below.
Additional guidelines:
* Use `$('element-id')` instead of `document.getElementById`. This function can * Use `$('element-id')` instead of `document.getElementById`. This function can
be imported from util.m.js. be imported from util.m.js.
* Use single-quotes instead of double-quotes for all strings. `clang-format` now * Use single-quotes instead of double-quotes for all strings.
handles this automatically. * `clang-format` now handles this automatically.
* Prefer `event.preventDefault()` to `return false` from event handlers. * Use ES5 getters and setters
* Use `@type` (instead of `@return` or `@param`) for JSDoc annotations on
getters/setters
* Prefer `event.preventDefault()` to `return false` from event handlers
* Prefer `this.addEventListener('foo-changed', this.onFooChanged_.bind(this));` * Prefer `this.addEventListener('foo-changed', this.onFooChanged_.bind(this));`
instead of always using an arrow function wrapper, when it makes the code less instead of always using an arrow function wrapper, when it makes the code less
@@ -341,56 +337,6 @@ if (!enterKey) {
errors. Instead use `assert()` statements. Only use the optional chaining errors. Instead use `assert()` statements. Only use the optional chaining
feature when the code needs to handle null/undefined gracefully. feature when the code needs to handle null/undefined gracefully.
* Don't use `async` if the body of the function does not use `await`. If the
function indeed needs to return a Promise:
```js
// Don't do this.
async function hello(): Promise<string> {
return 'Hello';
}
// Do this instead.
function hello(): Promise<string> {
return Promise.resolve('Hello');
}
```
* Don't use the non-null `!` operator unnecessarily, for example when TypeScript
already knows that a variable can't be null. For example:
```js
// Don't do this. querySelectorAll never returns null.
let items = document.body.querySelectorAll('div')!;
// Do this instead.
let items = document.body.querySelectorAll('div');
```
### ESLint checks
A big part of the styleguide is automatically enforced via ESLint checks. There
are two types of ESLint checks:
1. **Checks applied at presubmit time**.
These can be triggered locally with `git cl presubmit --files='path/to/folder/*.ts'`
and run as part of `git cl upload`.
2. **Checks applied at build time**. These are type-aware checks, which are more
sophisticated than the presubmit checks at #1 and must be run as part of the
build as they require a tsconfig file to work. See [this list](
https://typescript-eslint.io/rules/?=typeInformation) of all possible such
checks (not all of these are used in Chromium). Build-time ESLint checks can
be triggered locally by building the `chrome` or `browser_tests` binaries, or
by explicitly triggering the `:lint` target for cases where `build_webui()`
or `build_webui_tests()` is used. For example by running:
<br><br>
`autoninja -C out/chromium/ chrome/browser/resources/settings:lint`
<br><br>
See [`build_webui()` docs](
https://chromium.googlesource.com/chromium/src/+/HEAD/docs/webui/webui_build_configuration.md#build_webui)
for more details.
### Closure compiler (legacy ChromeOS Ash code only) ### Closure compiler (legacy ChromeOS Ash code only)
@@ -438,9 +384,6 @@ are two types of ESLint checks:
* `Promise` * `Promise`
* `Set` * `Set`
* Use ES5 getters and setters
* Use `@type` (instead of `@return` or `@param`) for JSDoc annotations on
getters/setters
## Polymer ## Polymer

@@ -449,7 +449,7 @@ export class CertificateManagerV2Element extends
} }
private onClientPlatformCertsLinkRowClick_(e: Event) { private async onClientPlatformCertsLinkRowClick_(e: Event) {
e.preventDefault(); e.preventDefault();
Router.getInstance().navigateTo(Page.PLATFORM_CLIENT_CERTS); Router.getInstance().navigateTo(Page.PLATFORM_CLIENT_CERTS);
} }

@@ -448,7 +448,7 @@ template("build_webui") {
} }
enable_type_aware_eslint_checks = enable_type_aware_eslint_checks =
!defined(invoker.enable_type_aware_eslint_checks) || defined(invoker.enable_type_aware_eslint_checks) &&
invoker.enable_type_aware_eslint_checks invoker.enable_type_aware_eslint_checks
if (enable_type_aware_eslint_checks) { if (enable_type_aware_eslint_checks) {
@@ -456,12 +456,10 @@ template("build_webui") {
in_folder = preprocess_dir in_folder = preprocess_dir
in_files = filter_exclude(ts_files, [ "*.js" ]) in_files = filter_exclude(ts_files, [ "*.js" ])
tsconfig = "$target_gen_dir/tsconfig_build_ts.json" tsconfig = "$target_gen_dir/tsconfig_build_ts.json"
deps = [ ":build_ts" ] deps = [
if (enable_source_maps) { ":build_ts",
deps += [ ":create_source_maps" ] ":preprocess_ts_files",
} else { ]
deps += [ ":preprocess_ts_files" ]
}
} }
} }