Skip to content

Commit 447a54f

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[npm] Replace npm run watch with npm run build -- --watch.
When `devtools_bundle = false` we can use the fast rebuild for `.ts` files as well (although this is unsound in case of `const enum`s), but that was already the case with the `watch_build.js` script. Also spit out a warning if using `--watch` and `devtools_bundle` isn't set to `false` in the GN args. Fixed: 411328609 Change-Id: I9d6f6b51be96b4af581e7c165045e9f0f47e7a0f Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6688491 Auto-Submit: Benedikt Meurer <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]> Reviewed-by: Ergün Erdoğmuş <[email protected]>
1 parent 44649d7 commit 447a54f

File tree

4 files changed

+29
-237
lines changed

4 files changed

+29
-237
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
"rdb": "rdb stream -new -realm chromium:public --",
3131
"start": "vpython3 third_party/node/node.py --output scripts/run_start.mjs",
3232
"webtest": "vpython3 third_party/node/node.py --output scripts/npm_test.js",
33-
"watch": "vpython3 third_party/node/node.py --output scripts/watch_build.js",
33+
"watch": "npm run build -- --watch",
3434
"test": "vpython3 third_party/node/node.py --output scripts/run_on_target.mjs gen/test/run.js"
3535
},
3636
"devDependencies": {

scripts/devtools_build.mjs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export class FeatureSet {
134134
*
135135
* @param {Error} error the `Error` from the failed `autoninja` invocation.
136136
* @param {string} outDir the absolute path to the `target` out directory.
137-
* @param {string} target the targe relative to `//out`.
137+
* @param {string} target the target relative to `//out`.
138138
* @return {string} the human readable error message.
139139
*/
140140
function buildErrorMessageForNinja(error, outDir, target) {
@@ -177,6 +177,7 @@ ${output}
177177
return `Failed to build \`${target}' in \`${outDir}' (${message.substring(0, message.indexOf('\n'))})`;
178178
}
179179

180+
/** @enum */
180181
export const BuildStep = {
181182
GN: 'gn',
182183
AUTONINJA: 'autoninja',
@@ -186,7 +187,7 @@ export class BuildError extends Error {
186187
/**
187188
* Constructs a new `BuildError` with the given parameters.
188189
*
189-
* @param {keyof BuildStep} step the build step that failed.
190+
* @param {BuildStep} step the build step that failed.
190191
* @param {Object} options additional options for the `BuildError`.
191192
* @param {Error} options.cause the actual cause for the build error.
192193
* @param {string} options.outDir the absolute path to the `target` out directory.
@@ -211,8 +212,8 @@ export class BuildError extends Error {
211212
*/
212213

213214
/**
214-
* @param {string} target
215-
* @return {Promise<void>}
215+
* @param {string} target the target relative to `//out`.
216+
* @return {Promise<Map<string, string>>} the GN args for the `target`.
216217
*/
217218
export async function prepareBuild(target) {
218219
const outDir = path.join(rootPath(), 'out', target);
@@ -229,6 +230,8 @@ export async function prepareBuild(target) {
229230
throw new BuildError(BuildStep.GN, {cause, outDir, target});
230231
}
231232
}
233+
234+
return await gnArgsForTarget(target);
232235
}
233236

234237
/** @type Map<string, Promise<Map<string, string>>> */
@@ -279,7 +282,9 @@ function gnRefsForTarget(target, filename) {
279282
}
280283

281284
async function computeBuildTargetsForFiles(target, filenames) {
282-
if (filenames && filenames.length && filenames.every(filename => path.extname(filename) === '.css')) {
285+
const SUPPORTED_EXTENSIONS = ['.css', '.ts'];
286+
if (filenames && filenames.length &&
287+
filenames.every(filename => SUPPORTED_EXTENSIONS.includes(path.extname(filename)))) {
283288
if (isInChromiumDirectory().isInChromium) {
284289
filenames = filenames.map(filename => path.join('third_party', 'devtools-frontend', 'src', filename));
285290
}
@@ -288,6 +293,11 @@ async function computeBuildTargetsForFiles(target, filenames) {
288293
try {
289294
const gnRefs = (await Promise.all(filenames.map(filename => gnRefsForTarget(target, filename)))).flat();
290295
if (gnRefs.length) {
296+
// If there are any changes to TypeScript files, we need to also rebuild the
297+
// `en-US.json`, as otherwise the changes to `UIStrings` aren't picked up.
298+
if (filenames.some(filename => path.extname(filename) === '.ts')) {
299+
gnRefs.push('collect_strings');
300+
}
291301
return gnRefs;
292302
}
293303
} catch (error) {

scripts/run_build.mjs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,19 @@ const timeFormatter = new Intl.NumberFormat('en-US', {
4545
// Prepare the build target if not initialized.
4646
const spinner = ora('Preparing…').start();
4747
try {
48-
await prepareBuild(target);
48+
const gnArgs = await prepareBuild(target);
49+
if (watch) {
50+
if (gnArgs.get('devtools_bundle') !== 'false') {
51+
spinner.info(
52+
'Using watch mode with full rebuilds. Use `gn gen out/' + target +
53+
' --args="devtools_bundle=false"` to enable fast rebuilds.');
54+
} else {
55+
spinner.warn(
56+
'Using watch mode with fast rebuilds (since `devtools_bundle=false`' +
57+
' for //out/' + target + '). Be aware that fast rebuilds are a best' +
58+
' effort and might not work reliably in all cases.');
59+
}
60+
}
4961
spinner.clear();
5062
} catch (error) {
5163
spinner.fail(error.message);

scripts/watch_build.js

Lines changed: 0 additions & 230 deletions
This file was deleted.

0 commit comments

Comments
 (0)