Fixes a bug where required fields were not prefixed with `!` in the
component bridges.
I also discovered that this doesn't play nicely with the union type
generation (!"fool"|"bar") doesn't make sense, but because we're not
going to support union types (as they don't convert cleanly into Closure
types) I've skipped those tests. I have a WIP CL that will strip out
this support (in favour for explicit errors telling people they can't
use union types in the Closure bridge code) so that CL will remove those
types.
Change-Id: Ibd2daf9520e492d975d38cd9e8dd2338cde6e009
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2349175
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Theme Support is currently in the ui/utils subdirectory. This ultimately
creates a circular dependency when we move to accessing it via imports
rather than by the global namespaced version. This CL creates an inert
copy of the Theme Support logic in the top level folder, and a future CL
will migrate all call sites to this version and remove the current
implementation in ui/utils.
Change-Id: I628b335dcc34fba29c79d11e4180593bb936f798
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2346370
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This CL introduces the parsing aspect of adding TS enum support to the component
bridges generation code. It _does not_ add full support for them, we still do
not output anything into the generated bridge, but I'm splitting it up into
smaller CLs.
This CL makes the tree walker understand and be aware of enums such that we can
then generate the relevant Closure doc comments (which will be in a follow-up
CL).
Change-Id: I5c91bb6bac291eef1aa99e685bbdee55e82ccd48
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2346369
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This CL fixes one particular case when the AST traversal wasn't thorough
enough; if you extend an interface, we need to also check the interface
that's being inherited from for any types that need to be pulled into
the bridge.
For example:
```
interface A {...}
interface B {
a: A
}
interface C extends B {
...
}
```
If the bridge generator decides that interface `C` should be in the
bridge, we need to walk through its parents (in this case, `B`) to check
for any other nested type references that should be included in the
bridge. In this case, because `B` references the `A` interface, we need
to add the `A` interface to the bridge.
Change-Id: Id211e2e02c12fed9ebb7c6456640bf87c3be70ac
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2339560
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
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>
The bridges were outputting functions as:
`x() {}`
But Clang would change that to:
```
x() {
}
```
It was a bit annoying that everytime the bridge file changed the
presubmit would fail as it reformatted, so this change brings the
bridges fully in line with Clang (e.g. a presubmit shouldn't make
changes to it).
Change-Id: Ie27a73b3d2933cf967c320d3c7c6122e4d6d806d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2332810
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Explicitly depend on the source-map-support package. Other packages
already used this indirectly so the files already exist in node_modules.
Add source-map-support/register to the require list for mocha which
means every e2e test will have source mapped error stacks by default.
Bug: 1104096
Change-Id: Id185ee76e82c100f1f763195673759f3f2319090
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2332222
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
The component bridge code would think `Object` is a type that it needs
to define as a Closure typedef but in fact it doesn't, it's built-in to
TS and maps to Closure's Object type. So if we find any, don't try to
define them to be converted, and instead output it directly as an
`Object` in Closure.
Change-Id: Ied707d131b95a8f595832f6e2080722f49522efb
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2329778
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
This CL fixes a bug in the bridges generation where nested types would
not correctly be added to the bridge when they were referenced via an
extended type.
For example, consider this code:
```
interface Detail {
id: number;
}
type NamedThing = {
name: string;
}
type Person = NamedThing & { details: Detail[] };
```
The bridges generator will correctly recognise that it needs to define a
`Person` typedef that includes all the members of `NamedThing` and also
the `details` field. But without this CL it will not realise that the
`Detail` interface is also referenced and therefore needs to be added to
the bridge.
This CL ensures when we extend types that we check their members for any
interfaces that are also required.
Change-Id: I5cf0de3ebc63dd56e71af8284ba6fe9fc10b396e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2320836
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@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>
Now the bridges generator supports extending types it also has to be
wary of types that override existing fields. For example:
```
type Person = { name: string, age: number};
type Jack = Person & { name: "jack" }
```
When we convert that to Closure, the bridges script needs to recognise
that the `Jack` type overrides the `name` field from Person, and
generate Closure that has:
```
* name:"jack"
* age:number
```
This CL makes that change by first collecting all members when we
extend, weeding out duplicates, and then converting them to Closure.
Change-Id: Iacd7fbf39844515257d2c331d6c4c4454b3bba60
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2318261
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
With this change we support:
```
type Person = {}
type SomeOtherPerson = Person & {...}
```
Closure doesn't have a concept of types being extended, so the bridges
code will instead parse all the types and generate a new type for
Closure made up of all the members of any types that are extended.
Note that this CL does _not_ add support for interfaces that extend
another, ONLY types. Interface extending is coming in a follow up CL.
Change-Id: I3f8253663430d16f28f1bdc3c40e582d8ff09248
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2318258
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
We previously only looked for interfaces, but sometimes we use types, so
now we support both.
There's still features that aren't supported - e.g. if you extend a
type, that won't work, but that's next on the list. Similarly we don't
parse union types to see if they contain other types, but that will also
be done in another follow-up CL. It's easier to incrementally add these
features than do them in one big go.
Change-Id: Ifa9c647b4b261a1ea1ca237d554733fed23221da
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2318247
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
If a setter took an object literal where the value was a union type
(e.g. `foo: Foo|null`) we would ignore it, and as such the bridge generator
would not include `Foo` in the bridge. This CL adds a check for union types
where we loop over each member and check if there's an interface in there.
Change-Id: I23039dea3af12d1bd0f3a3382e320a8c053cacc1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2317317
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
This CL fixes the component bridges so that it can find nested interfaces. That
is, if it sees:
```
interface Foo {
bar: Bar
}
```
Before this CL only the `Foo` interface would end up in the _bridge.js as a
Closure typedef. Now, with this CL, it will also include the `Bar` interface in
the _bridge.js file too.
Change-Id: I87a614fd5620c6572a062458a3e4b7dd30ca86e6
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2316066
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@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>
Jack Franklin
