When TypeScript writes to pre-existing files, it will overwrite the
contents, but it won't create a new file per se. If files in gen/ were
previously created by devtools_pre_built they will be hardlinked to the
original source file, thus any changes tsc makes to the file in gen will
be reflected back to the source. This causes an issue with ninja, since
it believes on the next run that the source file has changed.
This CL updates the behavior of devtools_pre_built such that it no
longer calls gn's copy, but rather a node utility that ensures that
there is a freshly minted copy of the file rather than a hardlink.
Change-Id: I11a23fce764101eb237e434a64159223ef8d700e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2335277
Auto-Submit: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Inside of devtools_entrypoint we use gn's copy command. This generates
hardlinks, which can sometimes mean that incremental builds get into a
broken state. This CL changes those copy commands over to being node
actions that ensure the files are copied rather than hardlinked, and it
also unlinks files before writing them (if they exist) to prevent the
case where hardlinked files are overwritten.
R=jacktfranklin@chromium.org
Change-Id: I86a1f351780afc3ac725c690866b4ac9dd649fd1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2335056
Auto-Submit: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
I had a bad gn file and running the cross-reference script didn't show
that but instead treated it as if gn knew of no dependencies and
therefore gave me a huge output of JS files that were not declared as gn
deps.
If the script gets an error, it should bail and log that error rather
than treat it as if there were no gn deps found.
Change-Id: I5672e97581659836adbf2fc1aa4fc5f837b78835
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2317308
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
It is possible for TypeScript to import a file that is not included in
the corresponding devtools_entrypoint as a dependency. In many cases
this would likely cause the build to fail, but it's also possible that a
dependency might have been provided as a dep of another entrypoint.
Given that ninja parallelizes builds this results in a race condition
where on some builds the dependency that wasn't declared is there, and
on some builds it is not. This is further complicated if build artifacts
from previous builds are kept around.
This CL introduces a script that can be run manually that cross
references the files that GN knows about, and the files that TypeScript
expects to be able to import. Any files expected by the TypeScript
compiler that are not declared in the BUILD.gn (even indirectly as a
dep of a dep etc) will be flagged.
Change-Id: Ieb95600f11bfc20e0e71d8792a7f344b13a0fb8e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2316063
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
The internal ninja copy action is, for Linux and Mac, the creation of a
hardlink. In the case of an is_debug = false build, we copy the
devtools_entrypoint file with ninja to the gen folder. However, should
the file then change in the gen folder, those changes will be reflected
back in the original source file since the two are hardlinked. This does
happen when the JavaScript file moves to being managed by the TypeScript
compiler, for example, because tsc often changes blank lines and adds
sourcemap information to the file. If the gen folder is empty and no
hardlink exists prior to build, there are no issues. If, on the other
hand, the hardlink exists, and the TypeScript compiler runs, the file
written in the gen folder will update, and then so will its original
source.
The ways to resolve this reflection back to the source folder is either
by deleting the gen folder before rebuilding (where one anticipates
changes being reflected in source), or, instead of using the ninja copy,
using an action to call out to a script that will ensure that a new file
is created and not hardlinked.
This CL chooses the latter path.
Change-Id: Id2cc8acdc240eb73ac736ddaf6f1eabdd08f8359
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2308534
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This CL ships the new elements breadcrumbs component to production,
replacing the old legacy breadcrumbs with the new custom element that
we've built to replace it.
Note that I haven't renamed the file from `NewElementsBreadcrumbs`. I wanted to
keep the diff on this CL clear and easy to follow. I plan to follow this up
with another CL that does the renaming.
Light mode & dark mode screenshots: https://imgur.com/a/UyF4ktY
Change-Id: I5c3df09456101ce03072f3eeb27f6a1c568cf9c7
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2297395
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Required changes:
- Add CodeMirror types as globals
- Add support for `is_web_worker` in `ts_library` and
`devtools_module`, as the FormatterWorker is a worker. This
also moves the `lib` specification out of `tsconfig.base.json`,
as we now need to dynamically generate it
- Fix (seemingly unrelated) issues with untyped events in other
files. I don't understand why TSC suddenly started to complain
about these.
DISABLE_THIRD_PARTY_CHECK=TypeScript fixes
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1098730
Change-Id: I7d33c22983f2fe5e793c20fa5d56411d996ec999
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2294985
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Sometimes in devtools we want to take a file directly from sources into
the `resources/inspector` folder, and other times we want to move files
from `out/Default/gen` into `resources/inspector`. Rather than have one
command for both of these and do nasty path wrangling to put the files
in exactly the right place, we instead split them into two templates.
This is nasty but our hope is we can hide this complexity behind other
templates that most people will use; this shouldn't be used directly by
people very often.
Change-Id: I3e7acbbed0c6d1bb8ad69a62cbe7341ad856d98b
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2287517
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
For folders that already have a `devtools_entrypoint` target, we
should skip Rollup as part of `build_release_applications.py`. To
remove the hardcoded list of skipped folders and enable
parallelization of a fixit to migrate all folders, we have to put
the information in the `module.json` instead.
Once all folders are migrated to `devtools_entrypoint`, we can remove
the field and the Rollup logic in `build_release_applications.py`.
R=aerotwist@chromium.org
Bug: 1101738
Change-Id: Ibadab13e87cf28622c40896a56f6ed18cc34bea2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2284838
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
This introduces the first TypeScript-authored file that also makes
use of TypeScript features. To do so, we have to perform several steps:
1. Move the `formatter_worker` typescript files into a new template called
`devtools_module`. This template takes care of running TypeScript and
copying the files it generates to `resources/inspector`. (The latter can
be removed when we are in a position to do so)
2. Move the files out of `all_devtools_modules` into (yet another) GN variable.
We should clean this up by using the `grd_file_group`, which is possible
with an integration with `devtools_module`. However, for simplicity's sake
I decided to fix that in a follow-up. The integration of `devtools_module`
with the GRD action script would then finally allow us to remove the duplication
of all of these file strings in multiple places.
3. To make sure that the buildgraph remains intact, we have to perform
the copy-step after the typescript-step. However, this means we need to use
a different name than `target_name` for the typescript compilation. We run
into issues there, as the project references assume that the names are the
full folder names (e.g. `../formatter_worker`). If we would then use a different
name for the `ts_library` action, then we generate a `tsconfig.json` with
the wrong name. To counteract that, introduce a (temporary)
`typescript_config_name` where we can explicitly set the name of the `tsconfig.json`
that we generate. This is not ideal and I am still thinking of a better
alternative, but haven't figured out a solution yet.
DISABLE_THIRD_PARTY_CHECK=Typescript fixes
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1098730
Change-Id: I1457067845cdedbc7d4ce6a80c12d7943b67087c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2282809
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
We will use this template later in a different `devtools_module` template.
It also clearly documents how the remapping works to `resources/inspector`.
Ideally, we will remove this copy-step altogether, but since not all
files for DevTools live in `gen/`, we can't make that change yet. But now
we have a central place where we do the copying, so hopefully it is easier
to remove that once we are there.
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1098730
Change-Id: I3981c97f99b3e3309ebe6ea0f4ec05fb0340835a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2282808
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Previously, we were generating the tsconfig only in release builds.
However, debug builds require the same treatment. Therefore,
refactor `devtools_entrypoint` to generate the tsconfig in both
cases.
We need to separate out the copying of the declaration file, as that
still only happens when we run Rollup in a release build.
This is a temporary fix to unbreak master and the inclusion of the
first custom element. We will clean up this logic in a follow-up CL.
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1098730, 1101738, 1061037
Change-Id: I231d87b6788b76c88028a80c27dd742e04dc70de
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2282518
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Before this change, we would incorrectly rename the generated tsconfig
as the second step of the devtools_entrypoint template. This would
then make TypeScript upset, as it was not able to find the built
Rollup bundle in release mode.
To fix that issue, we have to generate a custom `tsconfig.json`
(very much alike what we do for the third_party packages) and make
sure that the entrypoint has a corresponding `.d.ts` file as well.
CC=jacktfranklin@chromium.org
Bug: 1098730, 1101738, 1061037
Change-Id: I25e79e6c9d28377a752212c7380627d1c32ebb97
Also-By: aerotwist@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2279845
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Port formatter_worker to use devtools_entrypoint, which is the abstraction
around `rollup`. It handles the creation of the rollup bundle, but leaves
it alone if you are building with `is_debug=true`. This way, we can keep
development use the existing workflow where individual files are fetched,
while in release mode we bundle the entrypoint into 1 big file.
In the future, anything we consider an entrypoint must use this method.
This would include third_party packages like lit-html and CodeMirror.
A follow-up CL will move the lit-html entrypoint back into
third_party/lit-html, as we no longer require it to be a direct subfolder
of `front_end/` (that was fixed in
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2270168)
DISABLE_THIRD_PARTY_CHECK=TypeScript fixes
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1098730
Change-Id: I5d2e67cc9c71291e8b67bbc29354e75239aae9a9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2267001
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
This allows us to remove the pre-computation of the external modules
from the Python script, into the Rollup module. This is a necessary
change for the future, where we can define an entrypoint in any
location. This could be in a folder which is not a direct
sub-folder of `front_end`.
To do so, we operate on the assumption that the directory that the
input file is in, is the entrypoint. If you resolve an import that
is in that directory or in a subdirectory, we treat it as internal.
If it is outside of that folder, we treat it as external.
This largely mimics the implementation of the ESLint rule for
ES modules.
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Also-By: janscheffler@chromium.org
Also-By: alexrudenko@chromium.org
Bug: 1098730
Change-Id: I416b4c9cd9997b9fc2a50aea41077c41fea59af0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2270168
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Tim van der Lippe