WebUI: Support ESLint type aware checks in build_webui().
Type-aware ESLint checks (see [1], [2]) can do more sophisticated checks compared to non type-aware checks which run during presubmit. - Implement a new `eslint_ts()` GN rule along with corresponding tests. - Hook it up to `build_webui()`. - Guard it with a new `enable_type_aware_eslint_checks` flag that defaults to `false`. The new flag will be enabled on a target-by-target basis or as a whole once existing violations are fixed. Initially applying only @typescript-eslint/require-await when the flag is enabled. More checks will be added eventually. [1] https://typescript-eslint.io/rules/?=typeInformation [2] https://typescript-eslint.io/getting-started/typed-linting/ Bug: 394634491 Change-Id: Id2255656786fc144f95801f399bc34520c910cca Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5252273 Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/main@{#1417633}
This commit is contained in:
@ -290,6 +290,28 @@ the following:
|
||||
webui_context_type: See |webui_context_type| in webui_path_mappings().
|
||||
```
|
||||
|
||||
### **eslint_ts**
|
||||
|
||||
This rule makes it possible to run type-aware ESLint checks [1] on TypeScript
|
||||
files. Unlike the ESLint checks in `//tools/web_dev_style/eslint.config.mjs`
|
||||
which are run during presubmit, these checks require TS type information and
|
||||
have to run as part of the build.
|
||||
|
||||
The list of default checks that are applied are at
|
||||
`//ui/webui/resources/tools/eslint_ts.config_base.mjs` and are currently not
|
||||
configurable from callers.
|
||||
|
||||
[1] https://typescript-eslint.io/rules/?=typeInformation
|
||||
|
||||
#### **Arguments**
|
||||
```
|
||||
in_folder: The folder where all |in_files| reside.
|
||||
in_files: A list of TypeScript files to check. An assertion error is thrown if
|
||||
files other than '*.ts' are encountered.
|
||||
tsconfig: The TS config file to use during the checks. Usually this file is
|
||||
generated by a |ts_library()| target.
|
||||
```
|
||||
|
||||
### **bundle_js**
|
||||
|
||||
This rule is used to bundle larger user-facing WebUIs for improved performance.
|
||||
@ -522,25 +544,26 @@ rules described earlier.
|
||||
|
||||
Under the cover, build_webui() defines the following targets
|
||||
|
||||
* preprocess_if_expr("preprocess_ts_files")
|
||||
* preprocess_if_expr("preprocess_html_css_files")
|
||||
* create_js_source_maps("create_source_maps")
|
||||
* html_to_wrapper("html_wrapper_files")
|
||||
* css_to_wrapper("css_wrapper_files")
|
||||
* copy("copy_mojo")
|
||||
* webui_path_mappings("build_path_map")
|
||||
* ts_library("build_ts")
|
||||
* merge_js_source_maps("merge_source_maps")
|
||||
* bundle_js("build_bundle")
|
||||
* minify_js("build_min_js")
|
||||
* generate_grd("build_grd")
|
||||
* generate_grd("build_grdp")
|
||||
* grit("resources")
|
||||
1. `preprocess_if_expr("preprocess_ts_files")`
|
||||
2. `preprocess_if_expr("preprocess_html_css_files")`
|
||||
3. `create_js_source_maps("create_source_maps")`
|
||||
4. `html_to_wrapper("html_wrapper_files")`
|
||||
5. `css_to_wrapper("css_wrapper_files")`
|
||||
6. `copy("copy_mojo")`
|
||||
7. `webui_path_mappings("build_path_map")`
|
||||
8. `ts_library("build_ts")`
|
||||
9. `eslint_ts("lint")`
|
||||
10. `merge_js_source_maps("merge_source_maps")`
|
||||
11. `bundle_js("build_bundle")`
|
||||
12. `minify_js("build_min_js")`
|
||||
13. `generate_grd("build_grd")`
|
||||
14. `generate_grd("build_grdp")`
|
||||
15. `grit("resources")`
|
||||
|
||||
Some targets are only conditionally defined based on build_webui() input
|
||||
Some targets are only conditionally defined based on `build_webui()` input
|
||||
parameters.
|
||||
|
||||
Only ":build_ts", ":resources" and ":build_grdp" targets are public and can be
|
||||
Only `:build_ts`, `:resources` and `:build_grdp` targets are public and can be
|
||||
referred to from other parts of the build.
|
||||
|
||||
#### **Arguments**
|
||||
@ -643,6 +666,9 @@ 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
|
||||
eslint_ts() for more details.
|
||||
```
|
||||
|
||||
#### **Example**
|
||||
|
@ -2,23 +2,34 @@
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
|
||||
webui_sources = set([
|
||||
'bundle_js.py',
|
||||
'eslint_ts.py',
|
||||
'generate_grd.py',
|
||||
'minify_js.py',
|
||||
'rollup_plugin.mjs',
|
||||
])
|
||||
|
||||
webui_tests = set([
|
||||
'bundle_js_test.py',
|
||||
'eslint_ts_test.py',
|
||||
'generate_grd_test.py',
|
||||
'minify_js_test.py',
|
||||
])
|
||||
|
||||
def _CheckChangeOnUploadOrCommit(input_api, output_api):
|
||||
results = []
|
||||
webui_sources = set([
|
||||
'rollup_plugin.mjs', 'generate_grd.py', 'generate_grd_test.py',
|
||||
'minify_js.py', 'minify_js_test.py', 'bundle_js.py', 'bundle_js_test.py'
|
||||
])
|
||||
affected = input_api.AffectedFiles()
|
||||
affected_files = [input_api.os_path.basename(f.LocalPath()) for f in affected]
|
||||
if webui_sources.intersection(set(affected_files)):
|
||||
sources = webui_sources | webui_tests
|
||||
if sources.intersection(set(affected_files)):
|
||||
results += RunPresubmitTests(input_api, output_api)
|
||||
return results
|
||||
|
||||
|
||||
def RunPresubmitTests(input_api, output_api):
|
||||
presubmit_path = input_api.PresubmitLocalPath()
|
||||
sources = ['generate_grd_test.py', 'minify_js_test.py', 'bundle_js_test.py']
|
||||
tests = [input_api.os_path.join(presubmit_path, s) for s in sources]
|
||||
tests = [input_api.os_path.join(presubmit_path, s) for s in webui_tests]
|
||||
return input_api.canned_checks.RunUnitTests(input_api, output_api, tests)
|
||||
|
||||
|
||||
|
@ -15,6 +15,7 @@ import("//tools/typescript/ts_library.gni")
|
||||
import("//tools/typescript/webui_path_mappings.gni")
|
||||
import("//ui/webui/resources/tools/bundle_js.gni")
|
||||
import("//ui/webui/resources/tools/bundle_js_excludes.gni")
|
||||
import("//ui/webui/resources/tools/eslint_ts.gni")
|
||||
import("//ui/webui/resources/tools/generate_grd.gni")
|
||||
import("//ui/webui/resources/tools/minify_js.gni")
|
||||
import("//ui/webui/webui_features.gni")
|
||||
@ -170,17 +171,16 @@ template("build_webui") {
|
||||
|
||||
# Specifically the order in which these targets are executed is:
|
||||
#
|
||||
# 1) preprocess_if_expr()
|
||||
# 2) create_js_source_maps() (only if |invoker.enable_source_maps| flag is
|
||||
# true)
|
||||
# 3) html_to_wrapper(), css_to_wrapper()
|
||||
# 4) ts_library()
|
||||
# 5) merge_js_source_maps() (only if the |invoker.enable_source_maps| flag is
|
||||
# true)
|
||||
# 6) optimize_webui() (only if invoker.optimize && defined(invoker.optimize_webui_in_files))
|
||||
# 7) minify_js() (only if invoker.optimize && !defined(invoker.optimize_webui_in_files))
|
||||
# 8) generate_grd()
|
||||
# 9) grit()
|
||||
# 1) preprocess_if_expr()
|
||||
# 2) create_js_source_maps() (only if `invoker.enable_source_maps=true`)
|
||||
# 3) html_to_wrapper(), css_to_wrapper()
|
||||
# 4) ts_library()
|
||||
# 5) eslint_ts() (only if `invoker.enable_type_aware_eslint_checks=true`)
|
||||
# 6) merge_js_source_maps() (only if `invoker.enable_source_maps=true`)
|
||||
# 7) optimize_webui() (only if invoker.optimize && defined(invoker.optimize_webui_in_files))
|
||||
# 8) minify_js() (only if invoker.optimize && !defined(invoker.optimize_webui_in_files))
|
||||
# 9) generate_grd()
|
||||
# 10) grit()
|
||||
|
||||
if (defined(invoker.static_files)) {
|
||||
preprocess_if_expr("preprocess_static_files") {
|
||||
@ -200,6 +200,7 @@ template("build_webui") {
|
||||
visibility = [
|
||||
":build_ts",
|
||||
":html_wrapper_files",
|
||||
":lint",
|
||||
]
|
||||
if (enable_source_maps) {
|
||||
visibility += [ ":create_source_maps" ]
|
||||
@ -344,6 +345,7 @@ template("build_webui") {
|
||||
":$generate_grd_target_name",
|
||||
":build_bundle",
|
||||
":build_min_js",
|
||||
":lint",
|
||||
]
|
||||
|
||||
if (enable_source_maps) {
|
||||
@ -445,6 +447,22 @@ template("build_webui") {
|
||||
}
|
||||
}
|
||||
|
||||
enable_type_aware_eslint_checks =
|
||||
defined(invoker.enable_type_aware_eslint_checks) &&
|
||||
invoker.enable_type_aware_eslint_checks
|
||||
|
||||
if (enable_type_aware_eslint_checks) {
|
||||
eslint_ts("lint") {
|
||||
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",
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
if (enable_source_maps) {
|
||||
merge_js_source_maps("merge_source_maps") {
|
||||
deps = [ ":build_ts" ]
|
||||
@ -557,6 +575,12 @@ template("build_webui") {
|
||||
if (defined(invoker.grd_resource_path_prefix)) {
|
||||
resource_path_prefix = invoker.grd_resource_path_prefix
|
||||
}
|
||||
|
||||
if (enable_type_aware_eslint_checks) {
|
||||
# Adding as a dependency so that it runs as part of the build, even though
|
||||
# this `generate_grd()` target does not use any outputs from it.
|
||||
deps += [ ":lint" ]
|
||||
}
|
||||
}
|
||||
|
||||
if (!generate_grdp) {
|
||||
|
31
ui/webui/resources/tools/eslint_ts.config_base.mjs
Normal file
31
ui/webui/resources/tools/eslint_ts.config_base.mjs
Normal file
@ -0,0 +1,31 @@
|
||||
// Copyright 2025 The Chromium Authors
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
import typescriptEslint from '../../../../third_party/node/node_modules/@typescript-eslint/eslint-plugin/dist/index.js';
|
||||
import tsParser from '../../../../third_party/node/node_modules/@typescript-eslint/parser/dist/index.js';
|
||||
|
||||
export default {
|
||||
languageOptions: {
|
||||
ecmaVersion: 2020,
|
||||
sourceType: 'module',
|
||||
parser: tsParser,
|
||||
|
||||
// The following field should be specified by client code. as follows:
|
||||
//
|
||||
// parserOptions: {
|
||||
// project: [path.join(import.meta.dirname, './tsconfig_build_ts.json')],
|
||||
// },
|
||||
},
|
||||
|
||||
plugins: {
|
||||
'@typescript-eslint': typescriptEslint,
|
||||
},
|
||||
|
||||
files: ['**/*.ts'],
|
||||
|
||||
rules: {
|
||||
'require-await': 'off',
|
||||
'@typescript-eslint/require-await' : 'error',
|
||||
},
|
||||
};
|
51
ui/webui/resources/tools/eslint_ts.gni
Normal file
51
ui/webui/resources/tools/eslint_ts.gni
Normal file
@ -0,0 +1,51 @@
|
||||
# Copyright 2025 The Chromium Authors
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
|
||||
import("//third_party/node/node.gni")
|
||||
|
||||
# GN rule to run ESlint type-aware checks on TypeScript files. Unlike the ESLint
|
||||
# checks in tools/web_dev_style/eslint.config.mjs which are run on presubmit,
|
||||
# these checks require TS type information (provided in the form of a tsconfig
|
||||
# file) to run.
|
||||
|
||||
template("eslint_ts") {
|
||||
node(target_name) {
|
||||
script = "//ui/webui/resources/tools/eslint_ts.py"
|
||||
|
||||
forward_variables_from(invoker,
|
||||
[
|
||||
"deps",
|
||||
"in_folder",
|
||||
"in_files",
|
||||
"tsconfig",
|
||||
])
|
||||
|
||||
# The file holding the ESLint configuration.
|
||||
config_base = "//ui/webui/resources/tools/eslint_ts.config_base.mjs"
|
||||
|
||||
inputs = [
|
||||
config_base,
|
||||
tsconfig,
|
||||
]
|
||||
outputs = [ "$target_gen_dir/eslint.config.mjs" ]
|
||||
|
||||
foreach(f, in_files) {
|
||||
extension = get_path_info(f, "extension")
|
||||
assert(extension == "ts")
|
||||
inputs += [ "${in_folder}/${f}" ]
|
||||
}
|
||||
|
||||
args = [
|
||||
"--in_folder",
|
||||
rebase_path(in_folder, root_build_dir),
|
||||
"--out_folder",
|
||||
rebase_path(target_gen_dir, root_build_dir),
|
||||
"--config_base",
|
||||
rebase_path(config_base, target_gen_dir),
|
||||
"--tsconfig",
|
||||
rebase_path(tsconfig, target_gen_dir),
|
||||
"--in_files",
|
||||
] + in_files
|
||||
}
|
||||
}
|
78
ui/webui/resources/tools/eslint_ts.py
Executable file
78
ui/webui/resources/tools/eslint_ts.py
Executable file
@ -0,0 +1,78 @@
|
||||
#!/usr/bin/env python3
|
||||
# Copyright 2025 The Chromium Authors
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
|
||||
import argparse
|
||||
import sys
|
||||
import os
|
||||
|
||||
_HERE_PATH = os.path.dirname(__file__)
|
||||
_SRC_PATH = os.path.normpath(os.path.join(_HERE_PATH, '..', '..', '..', '..'))
|
||||
_CWD = os.getcwd()
|
||||
|
||||
_NODE_PATH = os.path.join(_SRC_PATH, 'third_party', 'node')
|
||||
sys.path.append(_NODE_PATH)
|
||||
import node
|
||||
import node_modules
|
||||
|
||||
_ESLINT_CONFIG_TEMPLATE = """import path from 'path';
|
||||
|
||||
import defaultConfig from '%(config_base)s';
|
||||
|
||||
export default [
|
||||
defaultConfig,
|
||||
{
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
'project': [path.join(import.meta.dirname, './%(tsconfig)s')],
|
||||
},
|
||||
},
|
||||
},
|
||||
];"""
|
||||
|
||||
# A subset of the 'X errors and Y warnings potentially fixable with the `--fix`
|
||||
# option.' instruction that is emitted by ESLint when automatically fixable
|
||||
# errors exist. This is used to strip this line from the error output to avoid
|
||||
# any confusion, when using the `--fix` flag wouldn't actually work because the
|
||||
# input files are generated files.
|
||||
_TOKEN_TO_STRIP = 'potentially fixable with the `--fix` option'
|
||||
|
||||
|
||||
def _generate_config_file(out_dir, config_base, tsconfig):
|
||||
config_file = os.path.join(out_dir, 'eslint.config.mjs')
|
||||
with open(config_file, 'w', newline='', encoding='utf-8') as f:
|
||||
f.write(_ESLINT_CONFIG_TEMPLATE % {
|
||||
'config_base': config_base,
|
||||
'tsconfig': tsconfig
|
||||
})
|
||||
return config_file
|
||||
|
||||
|
||||
def main(argv):
|
||||
parser = argparse.ArgumentParser()
|
||||
parser.add_argument('--in_folder', required=True)
|
||||
parser.add_argument('--out_folder', required=True)
|
||||
parser.add_argument('--config_base', required=True)
|
||||
parser.add_argument('--tsconfig', required=True)
|
||||
parser.add_argument('--in_files', nargs='*', required=True)
|
||||
|
||||
args = parser.parse_args(argv)
|
||||
|
||||
config_file = _generate_config_file(args.out_folder, args.config_base,
|
||||
args.tsconfig)
|
||||
if len(args.in_files) == 0:
|
||||
return
|
||||
|
||||
node.RunNode([
|
||||
node_modules.PathToEsLint(),
|
||||
# Force colored output, otherwise no colors appear when running locally.
|
||||
'--color',
|
||||
'--quiet',
|
||||
'--config',
|
||||
config_file,
|
||||
] + list(map(lambda f: os.path.join(args.in_folder, f), args.in_files)))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
main(sys.argv[1:])
|
66
ui/webui/resources/tools/eslint_ts_test.py
Executable file
66
ui/webui/resources/tools/eslint_ts_test.py
Executable file
@ -0,0 +1,66 @@
|
||||
#!/usr/bin/env python3
|
||||
# Copyright 2025 The Chromium Authors
|
||||
# Use of this source code is governed by a BSD-style license that can be
|
||||
# found in the LICENSE file.
|
||||
|
||||
import eslint_ts
|
||||
import os
|
||||
import tempfile
|
||||
import shutil
|
||||
import unittest
|
||||
|
||||
_HERE_DIR = os.path.dirname(__file__)
|
||||
|
||||
|
||||
class EslintTsTest(unittest.TestCase):
|
||||
|
||||
_in_folder = os.path.join(_HERE_DIR, "tests", "eslint_ts")
|
||||
|
||||
def setUp(self):
|
||||
self._out_dir = tempfile.mkdtemp(dir=self._in_folder)
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self._out_dir)
|
||||
|
||||
def _read_file(self, path):
|
||||
with open(path, "r", encoding="utf-8") as file:
|
||||
return file.read()
|
||||
|
||||
def _run_test(self, in_files):
|
||||
config_base = os.path.join(_HERE_DIR, "eslint_ts.config_base.mjs")
|
||||
tsconfig = os.path.join(self._in_folder, "tsconfig.json")
|
||||
|
||||
args = [
|
||||
"--in_folder",
|
||||
self._in_folder,
|
||||
"--out_folder",
|
||||
self._out_dir,
|
||||
"--config_base",
|
||||
os.path.relpath(config_base, self._out_dir).replace(os.sep, '/'),
|
||||
"--tsconfig",
|
||||
os.path.relpath(tsconfig, self._out_dir).replace(os.sep, '/'),
|
||||
"--in_files",
|
||||
*in_files,
|
||||
]
|
||||
|
||||
eslint_ts.main(args)
|
||||
|
||||
def testSuccess(self):
|
||||
self._run_test(["no_violations.ts"])
|
||||
actual_contents = self._read_file(
|
||||
os.path.join(self._out_dir, "eslint.config.mjs"))
|
||||
expected_contents = self._read_file(
|
||||
os.path.join(self._in_folder, "eslint_expected.config.mjs"))
|
||||
self.assertMultiLineEqual(expected_contents, actual_contents)
|
||||
|
||||
def testError(self):
|
||||
with self.assertRaises(RuntimeError) as context:
|
||||
self._run_test(["with_violations.ts"])
|
||||
|
||||
# Expected ESLint rule violation that should be part of the error output.
|
||||
_EXPECTED_STRING = "@typescript-eslint/require-await"
|
||||
self.assertTrue(_EXPECTED_STRING in str(context.exception))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
@ -0,0 +1,14 @@
|
||||
import path from 'path';
|
||||
|
||||
import defaultConfig from '../../../eslint_ts.config_base.mjs';
|
||||
|
||||
export default [
|
||||
defaultConfig,
|
||||
{
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
'project': [path.join(import.meta.dirname, './../tsconfig.json')],
|
||||
},
|
||||
},
|
||||
},
|
||||
];
|
10
ui/webui/resources/tools/tests/eslint_ts/no_violations.ts
Normal file
10
ui/webui/resources/tools/tests/eslint_ts/no_violations.ts
Normal file
@ -0,0 +1,10 @@
|
||||
// Copyright 2025 The Chromium Authors
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
const foo: number = 3;
|
||||
const bar: number = foo;
|
||||
|
||||
function foo() {
|
||||
return foo + bar;
|
||||
}
|
11
ui/webui/resources/tools/tests/eslint_ts/tsconfig.json
Normal file
11
ui/webui/resources/tools/tests/eslint_ts/tsconfig.json
Normal file
@ -0,0 +1,11 @@
|
||||
{
|
||||
"extends": "../../../../../../tools/typescript/tsconfig_base.json",
|
||||
"compilerOptions": {
|
||||
"rootDir": ".",
|
||||
"outDir": "tsc"
|
||||
},
|
||||
"files": [
|
||||
"no_violations.ts",
|
||||
"with_violations.ts"
|
||||
]
|
||||
}
|
11
ui/webui/resources/tools/tests/eslint_ts/with_violations.ts
Normal file
11
ui/webui/resources/tools/tests/eslint_ts/with_violations.ts
Normal file
@ -0,0 +1,11 @@
|
||||
// Copyright 2025 The Chromium Authors
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
const foo: number = 3;
|
||||
const bar: number = foo;
|
||||
|
||||
// Violating @typescript-eslint/require-await.
|
||||
async function foo() {
|
||||
return foo + bar;
|
||||
}
|
Reference in New Issue
Block a user