The current bundling system with Rollup tries to determine when given a
file if it’s an entry point (e.g. ui/ui.js) or not (e.g ui/someFile.js).
If it’s not an entry point, it will bundle that file into the entry
point JS file. So when we go to bundle ui/ui.js, it’ll find files like
ui/someFile.js, and pull them into ui/ui.js, which ends up being a fully
“rolled up” version of the ui folder.
The problem comes when you nest these folders;
ui/components/components.js is an entry point but the logic that the
Rollup bundling code uses incorrectly detects it as a regular file. So
you end up with ui/ui.js including a rolled-up version of
ui/components/components.js, which itself is a rolled-up file. We then
ship ui/ui.js and ui/components/components.js, meaning we’ve shipped
components.js twice - once as a standalone file, and once because it was
rolled up into ui.js.
This CL fixes this by changing how we detect bundles, we now look for
importing files whose parent directory is the same, so:
- components/components.js is a bundle
- but components/foo.js is not a bundle
By making this change we also now correctly detect third_party bundles,
with the exception of Acorn which is a special case, so we can lose the
checks for that in our Rollup config.
The logic here is somewhat duplicated between Rollup's config and the
ESLint rule plugin that we have; I plan on making a follow-up CL that
tries to define this logic in one place so if it changes in the future
we can update the code in one place and have the linting & rollup
update, plus we'll be able to use the ESLint tests as coverage to ensure
we've not broken our bundling.
Fixed: 1144123
Change-Id: Idd82a3ef69a09871867626a0d74266e25801243f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2534202
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Only set the unhandled rejection handler once per process.
We run the setup and teardown multiple times per-process now.
Add clearPuppeteerState() to unset the state between runs on
a parallel test runner. This is because the port changes
between successive runs in the same process.
Remove the hasShutdown logic from mocha_hooks. Also remove the
beforeExit hook which was causing this to get called at least
twice for every process. It already gets called in the ordinary
shutdown case via the afterAll() hook. We only need to call it
in the extraordinary case which is the SIGINT case. Now that we
do the shutdown multiple times per process, we don't care if we
already shutdown on this process or not. The only issue could be
if ctrl+c is sent during the shutdown, but then we are crashing
anyway.
Right now the default # of jobs is 1, so out bots will still run
in serial mode. We want to add parallel mode as an options for
local development while we iron out any last problems with this
approach.
You can test this locally with npm run e2etests -- --jobs=8.
We make use of global setup fixtures to only run one hosted
mode server and share it between all parallel runners. Each
runner still starts its own chrome and restarts it between
files, which is a bit inefficient but still a big improvement
compared to serial mode.
Split the hosted mode server out into its own file inside
conductor as it can be dealt with entirely separately, and
its state is not per-process like the chrome state, but only
per-main-process which launches the test runner sub-processes.
Bug: 1101784
Change-Id: Ic0c9be3559708fd5403a88882b9fb93257633de9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2288694
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
The ColorSwatch.ts component uses the `Common.Color.Color` interface.
The bridges generator does not support deeply nested interfaces. So this
CL adds special casing for interfaces that we know we might have to deal
with from "legacy land" and makes sure they still get outputted
correctly.
Currently we only allow nested interfaces that start with `Common.`,
because I'd like to avoid their use in the new world if possible, but we
can easily expand this if required.
Once I got the type being compiled correctly, I then realised that we
also needed to add the imports into the outputted file, so the code now
checks for usage of Common, finds the matching import, and pulls it
over.
Finally, I had to update ColorSwatch.ts. It used public properties, which the
bridge generator doesn't support, so I swapped it to private properties with
getters. I don't love this change, but I think that's better rather than invest
more time in the (temporary) bridge generator code.
Note: this bug made it in because there's a bug in the bridges PRESUBMIT that
means if it errors it doesn't fail the PRESUBMIT. I have another CL incoming to
fix that, but I need to fix the actual component first so that when I fix the
PRESUBMIT I don't just block CQ for everyone!
Change-Id: I7376b0b7bee78bfe9d106fad6e2233a2b04fcf42
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2502042
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
There previously were two special files that were handled
by `build_release_applications`: root.js and RuntimeInstantiator.js.
Both files are explicitly part of the startup process of DevTools
and its various entrypoints.
To remove the copying from `build_release_applications`, we have
to move these to the relevant `devtools_entrypoint`. Since a
`devtools_entrypoints` bundles all subdirectories, we can't keep
these files in `front_end/` directly. Instead, we move these files
to `startup/` to denote their special-casing in the startup process.
Next to that, we have to fix all usages of these files in the entrypoints.
For all JavaScript entrypoint files, all side-effect legacy files
that are loaded by `startup.js` (previously known as `root.js` and renamed
to prevent confusion with the `root/` module) are removed. All
"additional" legacy files that a particular entrypoint requires are
still loaded as-is.
The RuntimeInstantiator is moved to become an implementation detail
of `startup/`. Therefore, all of the usages that were previously
importing from `RuntimeInstantiator` now import via `startup.js`.
In the end, the special-casing of these files are removed and renamed
for clarity. In the future, we want to remove the complicated
entrypoints startup process, but we are not ready for that yet.
That will require additional cleanups with `resources` in `module.json`
before that change can happen.
R=aerotwist@chromium.org
Bug: 1131500
Change-Id: I198d5a62d2aab70f842c68d9b4c0871fde1587a4
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2485072
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
To continue to move away from `build_release_applications.py`, move
building `common-legacy.js` into `devtools_entrypoint`. In the end,
it will allow us to remove `_rollup_module` from
`build_release_applications.py`.
The logic in `build_release_applications.py` is updated to assume
a pregenerated `-legacy.js` file based on the `pre_generates_legacy`
option in the `module.json` file. Once all `-legacy.js are migrated,
we can remove this option once again.
To make sure that we Rollup properly, we should assume that an
entrypoint in the same folder is regarded as external. Otherwise,
we would rollup the contents of `common.js` into `common-legacy.js`,
which is not what we want.
R=aerotwist@chromium.org
Bug: 1131500
Change-Id: Idcda3e1c2436a0bebb36501523c8d586d6e86fac
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2450297
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
For all `_module.js` files, we currently copy all `modules` into
the `module.json` metadata. However, since then the Runtime got
updated to always load the entrypoint. This is possible, because
in release modes only the entrypoint exists and all other files
are removed.
Therefore, we can use an empty array to denote that solely the
entrypoint should be loaded by the Runtime. If however the module
has a legacy file, the Runtime needs to load that instead. (See
`Runtime._loadModules` for more information) Therefore, include
solely the legacy file to the metadata information in the
`_module.js` to load it.
Eventually, this will allow us to remove the files from the modules
array in the `module.json`, as only Closure would require that
information.
R=aerotwist@chromium.org
Bug: 1131500
Change-Id: Ie14bd55b29b356a39aad7e8ca2041c979bd1b2cb
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2461783
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
A recent TS config change elsewhere had caused these tests not to
compile with the error of:
```
error TS6307: File '/Users/jacktfranklin/src/devtools/devtools-frontend/scripts/component_bridges/value_for_type_node.ts' is not listed within the file list of project '/Users/jacktfranklin/src/devtools/devtools-frontend/test/unittests/scripts/component_bridges/tsconfig.json'.
```
The fix is to mark the dependency from the unit tests as a TS project
reference, and then generate output in the same place as the source, so
that import paths don't need to change.
This isn't ideal, and we would use ts_library if doing this now, but
this code pre-dates ts_library and also is only going to be around for
the length of the TypeScriptification work, so it doesn't feel worth the
effort to restructure it.
Change-Id: I9a924bf3e2ed945dd22ce233eda2c8c15f3193c6
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2466188
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Root cause:
i18n is designed to work with strings that are declared in a UIStrings
object on the SAME file where they are being used. To my best knowledge
this is done to improve performance when searching for a translation,
but in Devtools there are strings that live in module.json files and
are exposed to i18n via a ModuleUIStrings.js file.
Fix:
There is already a mechanism for this kind of templating in the i18n
library which exposes a small subset of translations that match a
specific pattern, the fix is to change this pattern to match
ModuleUIStrings and to also include this as part of the translation
resolution.
Change-Id: If44a84a9b5892558bf832e6d08ea36d1a8f4ddd1
Bug: 1136655
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2459638
Commit-Queue: Vidal Diazleal <vidorteg@microsoft.com>
Reviewed-by: Simon Zünd <szuend@chromium.org>
The current behavior enforces that the second argument passed to all
i18n.getLocalizedString is part of UIStrings structure, this is not
possible to enforce in the current shape as a reference is also a valid
scenario
-------- e.g-----------
const title1 = UIStrings.title1;
const title2 = UIStrings.title2;
function render(title) { i18n.getLocalizedString(str, title) }
-------- end e.g-----------
Also minor fix in the naming convention of the file (localizationV2checks)
Change-Id: I9e7536850f7bbb73b2e27f5906343b3b1dbfd82d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2451369
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Christy Chen <chrche@microsoft.com>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Vidal Diazleal <vidorteg@microsoft.com>
This CL addresses a few issues that will help make it possible
to build Chromium using Python 3.
Nothing in this CL should cause any functional changes, and
Python 3 is not required (indeed, won't even work yet), but
this CL will be needed to unblock other work.
See https://crrev.com/c/2333868 for the roll-up Chromium patch,
which also has multiple other dependencies.
Bug: 1112471
Change-Id: Ica8a5b2b24674e1abd267bcd558b7101a6da6fc5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2330718
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Jack Franklin