WebUI: Update documentation to reflect new path mapping logic
Bug: 1402829 Change-Id: I9173d83c5a1e8dc82dc5e4c73e3d9df974ce2da9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4210119 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/main@{#1099597}
This commit is contained in:
Rebekah Potter
committed by
Chromium LUCI CQ
Chromium LUCI CQ
parent
55464a79cc
commit
84a6b56d37
@ -195,8 +195,15 @@ deps: Specifies all other ts_library targets generating libraries that this
|
||||
extra_deps: Used to specify build targets generating the input files for TS
|
||||
compiler.
|
||||
path_mappings: Additional non-default path mappings for absolute imports. The
|
||||
absolute 'chrome://resources/' paths are already mapped by
|
||||
default.
|
||||
absolute 'chrome://resources/' paths are already mapped for any
|
||||
resources in libraries that are listed in |deps|; for example
|
||||
adding "//ui/webui/resources/cr_elements:build_ts" in deps will
|
||||
automatically add the mapping for imports from that library
|
||||
(e.g. 'chrome://resources/cr_elements/cr_button/cr_button.js').
|
||||
Important: Don't add path_mappings without also adding the
|
||||
ts_library() target(s) responsible for the files being mapped to
|
||||
deps! path_mappings without corresponding deps can result in
|
||||
flaky build errors.
|
||||
manifest_excludes: List of input files to exclude from the output
|
||||
the manifest file.
|
||||
```
|
||||
|
||||
@ -1255,135 +1255,6 @@ computing stability metrics.
|
||||
error message includes the name of a network, each network name will be its
|
||||
own signature.
|
||||
|
||||
## Common TypeScript build issue: Missing dependencies
|
||||
Similar to how builds can flakily fail when a C++ file adds an include without
|
||||
updating the DEPS file appropriately, builds can flakily (or consistently) fail
|
||||
if TypeScript code adds an import but doesn't update the dependencies for its
|
||||
`ts_library()` target to include the library that contains that import. This
|
||||
has caused confusion for both developers and sheriffs in the past.
|
||||
|
||||
### Example Failure
|
||||
The following is an example build flake that occurred due to the file
|
||||
`personalization_app.ts` adding an import of `colors_css_updater.js`, but not
|
||||
updating its dependencies appropriately:
|
||||
|
||||
```
|
||||
gen/ash/webui/personalization_app/resources/preprocessed/js/personalization_app.ts:38:39 - error TS2792: Cannot find module 'chrome://resources/cr_components/color_change_listener/colors_css_updater.js'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
|
||||
|
||||
38 import {startColorChangeUpdater} from 'chrome://resources/cr_components/color_change_listener/colors_css_updater.js';
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
|
||||
Found 1 error in gen/ash/webui/personalization_app/resources/preprocessed/js/personalization_app.ts:38
|
||||
```
|
||||
|
||||
### For Chromium Sheriffs
|
||||
If you see a failure like the one in the example, there is a high chance that
|
||||
the regression range given by automated tools will not include the CL that is
|
||||
the root cause of the failure. There are 2 possible approaches to take to fix
|
||||
the build. One is described below at "fixing the error" - typically these are 1
|
||||
line fixes, but do require a few steps to identify the exact fix. An
|
||||
alternative workaround is as follows:
|
||||
1. Note that the file that failed ("1 error in") is `personalization_app.ts`.
|
||||
Find this file in the repo: in this case, it was at
|
||||
`ash/webui/personalization_app/resources/js/personalization_app.ts`.
|
||||
2. Find the failed import in the repo (line 38, as noted by the bot failure).
|
||||
3. Use "Blame" in Chromium code search to find out what CL added this import
|
||||
line.
|
||||
4. Either contact the CL owner or try reverting the CL that made the addition.
|
||||
|
||||
### Fixing the error
|
||||
The [fix](https://chromium-review.googlesource.com/c/chromium/src/+/3952957)
|
||||
for this example was just 1 line and was identified as follows:
|
||||
|
||||
1. Observe from this failure that the module that can't be found is
|
||||
`chrome://resources/cr_components/color_change_listener/colors_css_updater.js`.
|
||||
2. Find `colors_css_updater.ts` in the repository at
|
||||
`ui/webui/resources/cr_components/color_change_listener/colors_css_updater.ts`.
|
||||
3. Find the BUILD.gn file that compiles this TS file. The BUILD.gn file will in
|
||||
most cases be in the same folder as the TS file or one of its ancestors. In
|
||||
this case, it was
|
||||
`ui/webui/resources/cr_components/color_change_listener/BUILD.gn`.
|
||||
4. Observe the target name for the `ts_library()` target that compiled the file
|
||||
is `"build_ts"`, so the full target path is
|
||||
`//ui/webui/resources/cr_components/color_change_listener:build_ts`.
|
||||
5. Observe that the file where the import failed is `personalization_app.ts`,
|
||||
which is `ash/webui/personalization_app/resourcesjs/personalization_app.ts` in
|
||||
the repo.
|
||||
6. Find the `ts_library` target that compiles `personalization_app.ts` at
|
||||
`ash/webui/personalization_app/resources/BUILD.gn`.
|
||||
7. Observe that this target doesn't have the
|
||||
`//ui/webui/resources/cr_components/color_change_listener:build_ts` target
|
||||
listed in `deps`. Add the missing dependency there.
|
||||
|
||||
Note that if `colors_css_updater.js` was actually checked into the repo as a
|
||||
JavaScript file, steps 3, 4, and 7 would be slightly different as follows:
|
||||
|
||||
3. Find the BUILD.gn file that either copies or generates a
|
||||
`colors_css_updater.d.ts`. Generally, this will contain a
|
||||
`ts_definitions()` target, where the JS file is either passed as an input,
|
||||
or a target copying the checked in definitions file is a dependency.
|
||||
4. Observe the name of the target - usually `"generate_definitions"`.
|
||||
7. Look for this target in the `extra_deps` of the `ts_library()` target that
|
||||
depends on it. Add it to `extra_deps` if it's missing.
|
||||
|
||||
### For developers - Prevent missing dependency build errors
|
||||
When adding a new import (e.g. `import {FooSharedClass} from 'chrome://resources/foo/foo_shared.js';`) to a TypeScript file in your project:
|
||||
1. If the file in the repo is TypeScript (e.g.
|
||||
`ui/webui/resources/foo/foo_shared.ts`), find which `ts_library()` target
|
||||
compiles this file.
|
||||
2. If, for example, `ui/webui/resources/foo/BUILD.gn` contains:
|
||||
`ts_library("build_ts")`, which has `foo_shared.ts` listed in its `in_files`,
|
||||
then add `//ui/webui/resources/foo:build_ts` to your `ts_library()` target's
|
||||
deps as follows:
|
||||
|
||||
```
|
||||
ts_library("build_ts") {
|
||||
root_dir = my_root_dir
|
||||
out_dir = "$target_gen_dir/tsc"
|
||||
tsconfig_base = "tsconfig_base.json"
|
||||
deps = [
|
||||
"//ui/webui/resources/js:build_ts",
|
||||
"//ui/webui/resources/foo:build_ts", # This line is new
|
||||
]
|
||||
in_files = my_project_ts_files
|
||||
}
|
||||
```
|
||||
|
||||
Alternatively:
|
||||
1. If the file in the repo is JavaScript (i.e.
|
||||
`ui/webui/resources/foo/foo_shared.js`), look for which `ts_definitions()`
|
||||
target generates the corresponding `.d.ts` file or depends on a target
|
||||
copying a manually checked in `foo_shared.d.ts` file.
|
||||
2. If, for example, `ui/webui/resources/foo/BUILD.gn` contains
|
||||
`ts_definitions("generate_definitions")`, which lists `foo_shared.js` in
|
||||
`js_files` or alternatively depends on `:copy_definitions` which copies
|
||||
`foo_shared.d.ts`, then add `//ui/webui/resources/foo:generate_definitions`
|
||||
to your `ts_library()` target's `extra_deps` as follows:
|
||||
|
||||
```
|
||||
ts_library("build_ts") {
|
||||
root_dir = my_root_dir
|
||||
out_dir = "$target_gen_dir/tsc"
|
||||
tsconfig_base = "tsconfig_base.json"
|
||||
deps = [ "//ui/webui/resources/js:build_ts" ]
|
||||
|
||||
# This line is new
|
||||
extra_deps = [ "//ui/webui/resources/foo:generate_definitions" ]
|
||||
|
||||
in_files = my_project_ts_files
|
||||
}
|
||||
```
|
||||
|
||||
Note: If using the `build_webui()` wrapper rule, add the new dependency to
|
||||
`ts_deps` (for a TypeScript file) or `ts_extra_deps` (for a JavaScript file
|
||||
with definitions).
|
||||
|
||||
Failure to follow these steps can lead to other developers hitting flaky build
|
||||
errors and/or having their unrelated CLs reverted by sheriffs who aren't always
|
||||
aware that the regression range given in automated tools may not contain the
|
||||
true culprit for TypeScript related build flakes.
|
||||
|
||||
## See also
|
||||
|
||||
* WebUI's C++ code follows the [Chromium C++ styleguide](../styleguide/c++/c++.md).
|
||||
|
||||
Reference in New Issue
Block a user