imported exist in the file system.
Initally, we wanted to create a `.d.ts` file for each CSS file to
ensure that the file being imported actually exists. However, when
ninja is building `tsc` uses the generated `tsconfig` file in `gen`.
In the `tsconfig.json` `files` list it will see the original TS file
that's in the source tree. When it goes to see the TS file and sees
the `import styles from 'foo.css.js'`. However, that file does not
exist in the files list of the entrypoint or the source tree, so TS
never finds it.
Instead we went with the approach of adding a `declare module` to type
all `.css.js` files and then use a lint rule to ensure that if you
import `foo.css.js` that there is a `foo.css` file in the tree.
Bug: 1106746
Change-Id: I8a65734bf4a9b359f8210b0011f14800c5d9426b
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2953260
Commit-Queue: Kriti Sapra <kritisapra@google.com>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
This CL lints that for a given component that all the references to
its tag name are the same.
```
class Foo extends HTMLElement {
// Check that this name
static litTagName = LitHtml.literal\`devtools-foo\`
}
// And this name
ComponentHelpers.CustomElements.defineComponent('devtools-foo', Foo);
declare global {
interface HTMLElementTagNameMap {
// And this one are the same
'devtools-foo': Foo
}
}
```
Bug: 1153077
Change-Id: I29694449cb37950d1a5ff5391779e16e462926e2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2897279
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This CL updates our ESLint rule for custom event names:
* It changes it from wanting kebab-case to allonewordnopunctuation
* It allows references to CustomEvent.eventName for times when it's
useful to define the event name as a static.
The CL therefore updates a variety of events through the codebase that
were kebab-case. go/building-ui-devtools has also been updated.
Fixed: 1176758
Change-Id: Ifbe9851bc2f6bbe9347ec886cc8c026248cc5c43
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2894389
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This CL adds an ESLint rule that bans the use of:
```
LitHtml.html`<devtools-foo>`
```
Because from now on we want to enforce:
```
LitHtml.html`<${Foo.litTagName}>`
```
I have disabled the rule in all locations where we do not yet do this,
and will be working to fix these problems over a series of CLs.
Bug: 1153077
Change-Id: I8d18243d0243ea1403d5d57dbeb32c8a9682d2dd
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2876969
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Prior to this fix, the wasmparser_worker entrypoint would include
specific files from third_party/wasmparser. However, since the
files were included as part of a separate package, rolling up the
target could cause issues. They should have been part of the
`sources` of the wasmparser_worker entrypoint, but they were instead
included as `deps`. Even better would be to not include specific
files from the wasmparser and instead use an entrypoint.
In the specific case reported in crbug.com/1203165, the wasmparser
implementation was updated. As such, GN reran
`third_party/wasmparser` and determined that
`entrypoints/wasmparser_worker:wasmparser_worker` required
recompilation (since one if its dependencies were updated. However,
since `entrypoints/wasmparser_worker:wasmparser_worker` wasn't
producing a different output, GN would determine that it wouldn't
have to run rollup. This conclusion is wrong and is an artifact of
the inclusion of specific files of `third_party/wasmparser` by
the entrypoint.
To fix this, we should rollup all relevant sources in
`third_party/wasmparser` instead. That way, whenever the wasmparser
implementation is updated, it will properly roll up its content
into its bundle, ready for consumption by the entrypoint.
R=jacktfranklin@chromium.org
Bug: 1203165
Change-Id: Ic29ddea0d1f8e953e11e71a6a0e4e65c5f0f1ad6
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2853559
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This CL adds a rule that ensures that any LitHtml.render call is
nested within a coordinator.write. It uses a fairly simple approach of
looking for a parent coordinator.write() call, but it should pick up most
cases.
Note this CL doesn't enable the rule; I'll do that in a follow-up. There
are 22 violations of the rule, so I will work on fixing those.
Bug: 1188116
Change-Id: Id73eaaa76db4685450086d2f77e7b2de322210d0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2799754
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
When source code is in nested folders, the es_modules_import rule
would incorrectly compute the containing folders from `front_end`.
Thus, if you would have two sibling folders in `front_end/panels`,
the rule would incorrectly determine that they are in the same
folder, while technically they are in a different folder.
Therefore, the logic is rewritten to compute the common prefix
of the two paths. If either one path ends up resolving to just
a file, we assume that they were both part of the same folder.
In other words: iff after removing the common prefix you still
are importing both paths across a folder boundary, they are
considered a different folder.
R=jacktfranklin@chromium.org
Bug: 1187573
Change-Id: Ib34583e258e005a124735e17e6a92e829eb37ef3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2773054
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
There are a handful of e2e tests which test regressions, and consequently
have a bug assiciation. This association was part of the test names in
the same way as we reference bugs for skipped tests. If such a test is
then skipped it needs to start with two crbug references. That's
confusing for the reader as well as a the skipped test lint check, for
which the regression bug references can lead to false negatives. This CL
introduces a new lint check that prevents bug references in non-skipped
tests with the same syntax as skipped tests.
Bug: none
Change-Id: Ibdf59091aba942f85424c1f12cc31160d9fe786a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2761357
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
This CL heuristically prevents i18nString calls during module
instantiation time. Translated strings are most likely not available
yet (depending which module is loaded), so usually i18nLazyString
should be used instead.
The check looks at all the AST ancestor nodes of a call expression
and checks that function declaration and similar nodes are somewhere
on the ancestor chain. This DOES NOT guarantee that no calls to
i18nString are made but it is a good heuristic and the lint check
found a couple of wrong use-sites already that have been fixed
before this CL landed.
R=petermarshall@chromium.org
Bug: chromium:1180760
Change-Id: I04199fc54935b88d142a793f407438a718a104d6
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2731567
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
LitHtml strongly discourages this approach and it's one that often can
be avoided using CSS custom properties that are set from JavaScript, so
we're going to try to avoid its usage creeping into our codebase. We can
revisit if we hit upon a case where it's really needed.
Bug: chromium:1166108
Change-Id: Ib8c60d8670707b8f0e239d911ec569c64a3b1846
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2626294
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Kriti Sapra