Skip to content

Commit 7099949

Browse files
Lightning00BladeDevtools-frontend LUCI CQ
authored andcommitted
[cleanup] Fix yargs type
Uses ParseSync else the result is Promise<T> | T. This give IDE better types to work with. Bug: none Change-Id: I4d6014aeccd7428ec03bfde90c5d728c6182c1f7 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6387381 Commit-Queue: Nikolay Vitkov <[email protected]> Reviewed-by: Jack Franklin <[email protected]>
1 parent fc4247c commit 7099949

File tree

4 files changed

+154
-98
lines changed

4 files changed

+154
-98
lines changed

scripts/deps/sync-build-gn-imports.js

Lines changed: 57 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -202,31 +202,30 @@ function parseSourceFileForImports(code, fileName) {
202202
* @returns {ComparisonResult}
203203
*/
204204
function compareDeps({buildGN, sourceCode}) {
205-
const sourceImportsWithFileNameRemoved =
206-
sourceCode.imports
207-
.map(importPath => {
208-
// If a file imports `../core/sdk/sdk.js`, in the BUILD.gn that is
209-
// listed as `../core/sdk:bundle`. In BUILD.gn DEPS you never depend on the
210-
// specific file, but the directory that file is in. Therefore we drop the
211-
// filename from the import.
212-
return path.dirname(importPath);
213-
})
214-
.filter(dirName => {
215-
// This caters for the case where the import is `import X from './Foo.js'`.
216-
// Any sibling import doesn't need to be a DEP in BUILD.GN as they are in
217-
// the same module, but we will check later that it's in the sources entry.
218-
return dirName !== '.';
219-
})
220-
.map(importPath => {
221-
// If we now have './helpers' we want to replace that with just
222-
// 'helpers'. We do this because in the BUILD.gn file the dep will be
223-
// listed as something like "helpers:bundle", so the starting "./" will
224-
// break comparisons if we keep it around.
225-
if (importPath.startsWith('./')) {
226-
return importPath.slice(2);
227-
}
228-
return importPath;
229-
});
205+
const sourceImportsWithFileNameRemoved = sourceCode.imports
206+
.map(importPath => {
207+
// If a file imports `../core/sdk/sdk.js`, in the BUILD.gn that is
208+
// listed as `../core/sdk:bundle`. In BUILD.gn DEPS you never depend on the
209+
// specific file, but the directory that file is in. Therefore we drop the
210+
// filename from the import.
211+
return path.dirname(importPath);
212+
})
213+
.filter(dirName => {
214+
// This caters for the case where the import is `import X from './Foo.js'`.
215+
// Any sibling import doesn't need to be a DEP in BUILD.GN as they are in
216+
// the same module, but we will check later that it's in the sources entry.
217+
return dirName !== '.';
218+
})
219+
.map(importPath => {
220+
// If we now have './helpers' we want to replace that with just
221+
// 'helpers'. We do this because in the BUILD.gn file the dep will be
222+
// listed as something like "helpers:bundle", so the starting "./" will
223+
// break comparisons if we keep it around.
224+
if (importPath.startsWith('./')) {
225+
return importPath.slice(2);
226+
}
227+
return importPath;
228+
});
230229

231230
// Now we have to find the BUILD.gn module that contains this source file
232231
// We check each section of the BUILD.gn and look for this file in the `sources` list.
@@ -239,7 +238,9 @@ function compareDeps({buildGN, sourceCode}) {
239238
});
240239

241240
if (!buildGNModule) {
242-
throw new Error(`Could not find module in BUILD.gn for ${sourceCode.filePath}`);
241+
throw new Error(
242+
`Could not find module in BUILD.gn for ${sourceCode.filePath}`,
243+
);
243244
}
244245

245246
// Special case: if we are linting an entrypoint, we have to find the
@@ -251,11 +252,15 @@ function compareDeps({buildGN, sourceCode}) {
251252
// We are going to use the dependencies from the relevant devtools_module
252253
// find the `deps = [':foo']` line, and return 'foo'
253254
const moduleDep = buildGNModule.deps[0].slice(1);
254-
buildGNModule = buildGN.find(buildModule => buildModule.moduleName === moduleDep);
255+
buildGNModule = buildGN.find(
256+
buildModule => buildModule.moduleName === moduleDep,
257+
);
255258
}
256259

257260
if (!buildGNModule) {
258-
throw new Error(`Could not find devtools_module in BUILD.gn for the devtools_entrypoint of ${sourceCode.filePath}`);
261+
throw new Error(
262+
`Could not find devtools_module in BUILD.gn for the devtools_entrypoint of ${sourceCode.filePath}`,
263+
);
259264
}
260265

261266
const buildGNDepsWithTargetRemoved = buildGNModule.deps.map(dep => {
@@ -334,30 +339,35 @@ function validateDirectory(dirPath) {
334339
const sourceFiles = directoryChildren.filter(child => {
335340
const isFile = fs.lstatSync(path.join(dirPath, child)).isFile();
336341
// TODO: longer term we may want to support .css files here too.
337-
return (isFile && path.extname(child) === '.ts') && !child.endsWith('.test.ts');
342+
return (isFile && path.extname(child) === '.ts' && !child.endsWith('.test.ts'));
338343
});
339344

340345
/** @type {ValidateDirectoryResult} */
341346
const result = {
342347
missingBuildGNDeps: [],
343348
// We assume that all BUILD.GN deps are unused, and as we find them in
344349
// source code files we remove them from this set.
345-
unusedBuildGNDeps: new Set(parsedBuildGN.flatMap(mod => {
346-
// We don't worry about any deps that start with a colon, we are only
347-
// interested in DEPS that are actual files.
348-
return mod.deps.filter(dep => !dep.startsWith(':')).map(dep => {
349-
// Drop the :bundle part from a dep, otherwise we can't compare it
350-
// against the import statements from the source code.
351-
const withoutBundle = dep.split(':')[0];
352-
return withoutBundle;
353-
});
354-
}))
350+
unusedBuildGNDeps: new Set(
351+
parsedBuildGN.flatMap(mod => {
352+
// We don't worry about any deps that start with a colon, we are only
353+
// interested in DEPS that are actual files.
354+
return mod.deps.filter(dep => !dep.startsWith(':')).map(dep => {
355+
// Drop the :bundle part from a dep, otherwise we can't compare it
356+
// against the import statements from the source code.
357+
const withoutBundle = dep.split(':')[0];
358+
return withoutBundle;
359+
});
360+
}),
361+
),
355362
};
356363

357364
for (const sourceFile of sourceFiles) {
358365
const sourceCode = fs.readFileSync(path.join(dirPath, sourceFile), 'utf8');
359366
const parsedSource = parseSourceFileForImports(sourceCode, sourceFile);
360-
const diffWithGN = compareDeps({buildGN: parsedBuildGN, sourceCode: parsedSource});
367+
const diffWithGN = compareDeps({
368+
buildGN: parsedBuildGN,
369+
sourceCode: parsedSource,
370+
});
361371
if (!diffWithGN) {
362372
continue;
363373
}
@@ -377,15 +387,19 @@ module.exports = {
377387
compareDeps,
378388
parseBuildGN,
379389
parseSourceFileForImports,
380-
validateDirectory
390+
validateDirectory,
381391
};
382392

383393
// If invoked as CLI
384394
if (require.main === module) {
385395
const yargs = require('yargs')
386-
.option('directory', {type: 'string', desc: 'The directory to validate', demandOption: true})
396+
.option('directory', {
397+
type: 'string',
398+
desc: 'The directory to validate',
399+
demandOption: true,
400+
})
387401
.strict()
388-
.argv;
402+
.parseSync();
389403

390404
const directory = path.join(process.cwd(), yargs.directory);
391405
const result = validateDirectory(directory);

scripts/migration/web-tests-esm/rename-legacy-global.mjs

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,29 @@ import * as fs from 'fs/promises';
1414
import * as path from 'path';
1515
import yargs from 'yargs';
1616

17-
const yargsObject = yargs(process.argv.slice(2), process.cwd()).option('web-test-directory', {
18-
type: 'string',
19-
demandOption: true,
20-
})
21-
.option('import', {
22-
type: 'string',
23-
demandOption: true,
24-
})
25-
.option('from', {
26-
type: 'string',
27-
demandOption: true,
28-
})
29-
.option('to', {
30-
type: 'string',
31-
demandOption: true,
32-
})
33-
.strict()
34-
.argv;
17+
const yargsObject = yargs(process.argv.slice(2), process.cwd())
18+
.option('web-test-directory', {
19+
type: 'string',
20+
demandOption: true,
21+
})
22+
.option('import', {
23+
type: 'string',
24+
demandOption: true,
25+
})
26+
.option('from', {
27+
type: 'string',
28+
demandOption: true,
29+
})
30+
.option('to', {
31+
type: 'string',
32+
demandOption: true,
33+
})
34+
.strict()
35+
.parseSync();
3536

3637
// `createPlainTextSearchRegex` is stolen from string-utilities.ts.
3738
const SPECIAL_REGEX_CHARACTERS = '^[]{}()\\.^$*+?|-,';
38-
const createPlainTextSearchRegex = function(query) {
39+
const createPlainTextSearchRegex = function (query) {
3940
// This should be kept the same as the one in StringUtil.cpp.
4041
let regex = '';
4142
for (let i = 0; i < query.length; ++i) {
@@ -56,7 +57,9 @@ async function findWebTests(dir) {
5657
const entries = await fs.readdir(dir, { withFileTypes: true });
5758
const pathPromises = entries.map(async entry => {
5859
const p = path.resolve(dir, entry.name);
59-
if (entry.isDirectory() && entry.name !== 'resources') {return findWebTests(p);}
60+
if (entry.isDirectory() && entry.name !== 'resources') {
61+
return findWebTests(p);
62+
}
6063
if (entry.isFile() && entry.name.endsWith('.js')) {
6164
return [p];
6265
}
@@ -71,7 +74,9 @@ async function findWebTests(dir) {
7174
* it to an existing one.
7275
*/
7376
function addImportIfNecessary(content, importLine) {
74-
if (content.includes(importLine)) {return content;}
77+
if (content.includes(importLine)) {
78+
return content;
79+
}
7580

7681
// Find where to insert `importLine`. We'll add it either to
7782
// an existing import * as block or start a new one below the import {TestRunner} block.
@@ -87,7 +92,9 @@ function addImportIfNecessary(content, importLine) {
8792
}
8893
}
8994

90-
const linesToInsert = onlyHasTestRunnerImports ? ['', importLine] : [importLine];
95+
const linesToInsert = onlyHasTestRunnerImports
96+
? ['', importLine]
97+
: [importLine];
9198
lines.splice(lastImportIndex + 1, 0, ...linesToInsert);
9299
return lines.join('\n');
93100
}
@@ -96,7 +103,7 @@ const fromRegex = createPlainTextSearchRegex(yargsObject.from);
96103
const webTestPaths = await findWebTests(yargsObject.webTestDirectory);
97104

98105
for (const filePath of webTestPaths) {
99-
const content = await fs.readFile(filePath, { encoding: 'utf-8'});
106+
const content = await fs.readFile(filePath, { encoding: 'utf-8' });
100107

101108
let replacedContent = content;
102109
if (yargsObject.from === yargsObject.to && !content.match(fromRegex)) {
@@ -105,11 +112,13 @@ for (const filePath of webTestPaths) {
105112
continue;
106113
} else if (yargsObject.from !== yargsObject.to) {
107114
replacedContent = content.replaceAll(fromRegex, yargsObject.to);
108-
if (replacedContent === content) {continue;}
115+
if (replacedContent === content) {
116+
continue;
117+
}
109118
}
110119

111120
replacedContent = addImportIfNecessary(replacedContent, yargsObject.import);
112-
await fs.writeFile(filePath, replacedContent, {encoding: 'utf-8'});
121+
await fs.writeFile(filePath, replacedContent, { encoding: 'utf-8' });
113122
}
114123

115124
const suggestedCommitMessage = `

scripts/run_on_target.mjs

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,17 @@ import yargs from 'yargs';
99
import unparse from 'yargs-unparser';
1010

1111
const argv = yargs(process.argv.slice(2))
12-
.command('$0 [script]')
13-
.option('target', {alias: 't', type: 'string', default: 'Default'})
14-
.help(false)
15-
.version(false)
16-
.argv;
12+
.command('$0 [script]')
13+
.option('target', { alias: 't', type: 'string', default: 'Default' })
14+
.help(false)
15+
.version(false)
16+
.parseSync();
1717

18-
const target = argv['target'];
19-
let script = argv['script'];
18+
const target = argv.target;
19+
let script = argv.script;
2020

21-
delete argv['target'];
22-
delete argv['t'];
23-
delete argv['script'];
21+
delete argv.target;
22+
delete argv.script;
2423

2524
let sourceRoot = path.dirname(path.dirname(path.resolve(argv['$0'])));
2625

@@ -33,8 +32,13 @@ let cwd = path.join(sourceRoot, 'out', target);
3332

3433
if (!fs.existsSync(cwd)) {
3534
// Check if we are in a Chromium checkout and look for the out folder there.
36-
const maybeChromiumRoot = path.dirname(path.dirname(path.dirname(sourceRoot)));
37-
if (sourceRoot === path.join(maybeChromiumRoot, 'third_party', 'devtools-frontend', 'src')) {
35+
const maybeChromiumRoot = path.dirname(
36+
path.dirname(path.dirname(sourceRoot)),
37+
);
38+
if (
39+
sourceRoot ===
40+
path.join(maybeChromiumRoot, 'third_party', 'devtools-frontend', 'src')
41+
) {
3842
sourceRoot = maybeChromiumRoot;
3943
cwd = path.join(sourceRoot, 'out', target);
4044
const pathParts = script.split(path.sep);
@@ -46,15 +50,24 @@ if (!fs.existsSync(cwd)) {
4650
}
4751
}
4852

49-
if (!fs.existsSync(cwd) || !fs.statSync(cwd).isDirectory() || !fs.existsSync(path.join(cwd, 'build.ninja'))) {
53+
if (
54+
!fs.existsSync(cwd) ||
55+
!fs.statSync(cwd).isDirectory() ||
56+
!fs.existsSync(path.join(cwd, 'build.ninja'))
57+
) {
5058
console.error(
51-
`Target path ${cwd} does not exist or is not a directory. Please run 'gn gen out/${target}' first.`);
59+
`Target path ${cwd} does not exist or is not a directory. Please run 'gn gen out/${target}' first.`,
60+
);
5261
process.exit(1);
5362
}
5463
const scriptPath = path.resolve(cwd, script);
5564
if (!fs.existsSync(scriptPath)) {
5665
console.error(`Script path ${scriptPath} does not exist, trying ninja...`);
57-
const {error, status} = childProcess.spawnSync('autoninja', ['-C', cwd, script], {stdio: 'inherit', cwd: sourceRoot});
66+
const { error, status } = childProcess.spawnSync(
67+
'autoninja',
68+
['-C', cwd, script],
69+
{ stdio: 'inherit', cwd: sourceRoot },
70+
);
5871
if (error) {
5972
throw error;
6073
}
@@ -63,6 +76,10 @@ if (!fs.existsSync(scriptPath)) {
6376
}
6477
}
6578

66-
const {argv0} = process;
67-
const {status} = childProcess.spawnSync(argv0, [scriptPath, ...unparse(argv)], {stdio: 'inherit', env});
79+
const { argv0 } = process;
80+
const { status } = childProcess.spawnSync(
81+
argv0,
82+
[scriptPath, ...unparse(argv)],
83+
{ stdio: 'inherit', env },
84+
);
6885
process.exit(status);

0 commit comments

Comments
 (0)