A previous CL fixed a bug where type references in Closure typedefs were not
prefixed with ! correctly. However, that fix was a little eager, and started
prefixing functions and nullable types with ! too, which is incorrect.
This CL fixes that and adds some test cases, as the fact that the previous CL
landed is a clear sign we were missing test coverage of those specific cases.
Change-Id: I17bde9138fd04b0e2b6087a1008d99b603884e8b
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2354095
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>
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>
Jack Franklin
