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}
This commit is contained in:
chrome/browser/resources/pdf/ink
components/webui/flags/resources
content/browser/resources
docs/webui
styleguide/web
ui/webui/resources
@ -36,7 +36,7 @@ export class InkApi {
|
||||
return this.canvas_.getPDFDestructive();
|
||||
}
|
||||
|
||||
async setCamera(camera: drawings.Box) {
|
||||
setCamera(camera: drawings.Box) {
|
||||
this.camera_ = camera;
|
||||
this.canvas_.setCamera(camera);
|
||||
// Wait for the next task to avoid a race where Ink drops the camera value
|
||||
|
@ -400,6 +400,7 @@ export class FlagsAppElement extends CrLitElement {
|
||||
* don't actually exist until after the template code runs; normal navigation
|
||||
* therefore doesn't work.
|
||||
*/
|
||||
// eslint-disable-next-line @typescript-eslint/require-await
|
||||
private async highlightReferencedFlag() {
|
||||
if (!window.location.hash) {
|
||||
return;
|
||||
|
@ -267,7 +267,7 @@ async function renderUsageAndQuotaStats() {
|
||||
}
|
||||
}
|
||||
|
||||
async function renderSimulateStoragePressureButton() {
|
||||
function renderSimulateStoragePressureButton() {
|
||||
getProxy().isSimulateStoragePressureAvailable().then(result => {
|
||||
if (!result.available) {
|
||||
document.body
|
||||
|
@ -19,7 +19,7 @@ import type {XrFrameStatistics, XrLogMessage} from './xr_session.mojom-webui.js'
|
||||
|
||||
let browserProxy: BrowserProxy;
|
||||
|
||||
async function bootstrap() {
|
||||
function bootstrap() {
|
||||
browserProxy = BrowserProxy.getInstance();
|
||||
assert(browserProxy);
|
||||
|
||||
@ -30,7 +30,7 @@ async function bootstrap() {
|
||||
renderSessionStatisticsContent();
|
||||
}
|
||||
|
||||
async function setupSidebarButtonListeners() {
|
||||
function setupSidebarButtonListeners() {
|
||||
const deviceInfoButton = getRequiredElement('device-info-button');
|
||||
const sessionInfoButton = getRequiredElement('session-info-button');
|
||||
const runtimeInfoButton = getRequiredElement('runtime-info-button');
|
||||
@ -81,7 +81,7 @@ async function renderDeviceInfoContent() {
|
||||
deviceInfoContent.appendChild(table);
|
||||
}
|
||||
|
||||
async function renderSessionInfoContent() {
|
||||
function renderSessionInfoContent() {
|
||||
const sessionInfoContent = getRequiredElement('session-info-content');
|
||||
assert(sessionInfoContent);
|
||||
|
||||
@ -116,7 +116,7 @@ async function renderActiveRuntimesTable(
|
||||
runtimeInfoTable.recreateActiveRuntimesTable(activeRuntimes);
|
||||
}
|
||||
|
||||
async function renderRuntimeInfoContent() {
|
||||
function renderRuntimeInfoContent() {
|
||||
const runtimeInfoContent = getRequiredElement('runtime-info-content');
|
||||
assert(runtimeInfoContent);
|
||||
|
||||
@ -143,7 +143,7 @@ async function renderRuntimeInfoContent() {
|
||||
runtimeInfoContent.appendChild(runtimeChangelogTable);
|
||||
}
|
||||
|
||||
async function renderSessionStatisticsContent() {
|
||||
function renderSessionStatisticsContent() {
|
||||
const sessionStatisticsContent =
|
||||
getRequiredElement('session-statistics-content');
|
||||
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
|
||||
few underlying targets. See ts_library()'s
|
||||
|enable_source_maps| for more details.
|
||||
enable_type_aware_eslint_checks: Defaults to "false". Setting it to "true" turns
|
||||
on additional type-aware ESLint checks. See
|
||||
enable_type_aware_eslint_checks: Defaults to "true". Turns on additional
|
||||
type-aware ESLint checks. See
|
||||
eslint_ts() for more details.
|
||||
```
|
||||
|
||||
|
@ -302,21 +302,25 @@ JavaScript, but it is expected that all code should migrate to TS eventually.
|
||||
|
||||
### Style
|
||||
|
||||
See the [Google TypeScript Style
|
||||
Guide](https://google.github.io/styleguide/tsguide.html) as well as
|
||||
[ECMAScript Features in Chromium](es.md).
|
||||
The Chromium styleguide is combination of 3 components:
|
||||
|
||||
1. The [Google TypeScript Style Guide](
|
||||
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
|
||||
be imported from util.m.js.
|
||||
|
||||
* Use single-quotes instead of double-quotes for all strings.
|
||||
* `clang-format` now handles this automatically.
|
||||
* Use single-quotes instead of double-quotes for all strings. `clang-format` now
|
||||
handles this automatically.
|
||||
|
||||
* 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 `event.preventDefault()` to `return false` from event handlers.
|
||||
|
||||
* Prefer `this.addEventListener('foo-changed', this.onFooChanged_.bind(this));`
|
||||
instead of always using an arrow function wrapper, when it makes the code less
|
||||
@ -337,6 +341,56 @@ if (!enterKey) {
|
||||
errors. Instead use `assert()` statements. Only use the optional chaining
|
||||
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)
|
||||
|
||||
@ -384,6 +438,9 @@ if (!enterKey) {
|
||||
* `Promise`
|
||||
* `Set`
|
||||
|
||||
* Use ES5 getters and setters
|
||||
* Use `@type` (instead of `@return` or `@param`) for JSDoc annotations on
|
||||
getters/setters
|
||||
|
||||
## Polymer
|
||||
|
||||
|
@ -449,7 +449,7 @@ export class CertificateManagerV2Element extends
|
||||
}
|
||||
|
||||
|
||||
private async onClientPlatformCertsLinkRowClick_(e: Event) {
|
||||
private onClientPlatformCertsLinkRowClick_(e: Event) {
|
||||
e.preventDefault();
|
||||
Router.getInstance().navigateTo(Page.PLATFORM_CLIENT_CERTS);
|
||||
}
|
||||
|
@ -448,7 +448,7 @@ template("build_webui") {
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
if (enable_type_aware_eslint_checks) {
|
||||
@ -456,10 +456,12 @@ template("build_webui") {
|
||||
in_folder = preprocess_dir
|
||||
in_files = filter_exclude(ts_files, [ "*.js" ])
|
||||
tsconfig = "$target_gen_dir/tsconfig_build_ts.json"
|
||||
deps = [
|
||||
":build_ts",
|
||||
":preprocess_ts_files",
|
||||
]
|
||||
deps = [ ":build_ts" ]
|
||||
if (enable_source_maps) {
|
||||
deps += [ ":create_source_maps" ]
|
||||
} else {
|
||||
deps += [ ":preprocess_ts_files" ]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user