Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ 🏗 Fix typing in the build-system #34162

Merged
merged 7 commits into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion build-system/tasks/check-sourcemaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ function checkSourcemapSources(sourcemapJson, map) {
log(red('ERROR:'), 'Could not find', cyan('sources'));
throw new Error('Could not find sources array');
}
/** @type {string[]} */
const invalidSources = sourcemapJson.sources
.filter((source) => !source.match(/\[.*\]/)) // Ignore non-path sources '[...]'
.filter((source) => !fs.existsSync(source)); // All source paths should exist
Expand Down Expand Up @@ -133,7 +134,8 @@ function checkSourcemapMappings(sourcemapJson, map) {
// Zeroth sub-array corresponds to ';' and has no mappings.
// See https://www.npmjs.com/package/sourcemap-codec#usage
const firstLineMapping = decode(sourcemapJson.mappings)[1][0];
const [, sourceIndex, sourceCodeLine, sourceCodeColumn] = firstLineMapping;
const [, sourceIndex = 0, sourceCodeLine = 0, sourceCodeColumn] =
firstLineMapping;

const firstLineFile = sourcemapJson.sources[sourceIndex];
const contents = fs.readFileSync(firstLineFile, 'utf8').split('\n');
Expand Down
3 changes: 2 additions & 1 deletion build-system/tasks/e2e/describes-e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ function getFirefoxArgs(config) {
* @typedef {{
* browsers: (!Array<string>|undefined),
* environments: (!Array<!AmpdocEnvironment>|undefined),
* experiments: (!Array<string>|undefined),
* testUrl: string|undefined,
* fixture: string,
* initialRect: ({width: number, height:number}|undefined),
Expand Down Expand Up @@ -664,7 +665,7 @@ function getDriver({headless = false}, browserName, deviceName) {
*/
async function setUpTest(
{environment, ampDriver, controller},
{testUrl, version, experiments = [], initialRect}
{testUrl = '', version, experiments = [], initialRect}
) {
const url = new URL(testUrl);

Expand Down
14 changes: 7 additions & 7 deletions build-system/tasks/e2e/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function getLastExpectError() {
/**
* @param {*} actual
* @param {string=} opt_message
* @return {!ExpectStatic}
* @return {!chai.ExpectStatic}
*/
function expect(actual, opt_message) {
if (!installed) {
Expand Down Expand Up @@ -169,8 +169,8 @@ function installWrappers(chai, utils) {
}

/**
* @param {Chai.ChaiUtils} utils
* @return {Function(_super: Chai.AssertionStatic): Function(): any}
* @param {chai.ChaiUtils} utils
* @return {function(chai.AssertionStatic): function(): any}
*/
function overwriteAlwaysUseSuper(utils) {
const {flag} = utils;
Expand Down Expand Up @@ -228,8 +228,8 @@ function overwriteAlwaysUseSuper(utils) {
}

/**
* @param {Chai.AssertionStatic} _super
* @return {Function(): *}
* @param {chai.AssertionStatic} _super
* @return {function(): *}
*/
function inheritChainingBehavior(_super) {
return function () {
Expand All @@ -238,8 +238,8 @@ function inheritChainingBehavior(_super) {
}

/**
* @param {Chai.AssertionStatic} _super
* @return {Function(): *}
* @param {chai.AssertionStatic} _super
* @return {function(): *}
*/
function overwriteUnsupported(_super) {
return function () {
Expand Down
44 changes: 23 additions & 21 deletions build-system/tasks/e2e/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,27 +128,29 @@ async function fetchCoverage_(outDir) {
const zipFilename = path.join(outDir, 'coverage.zip');
const zipFile = fs.createWriteStream(zipFilename);

await new Promise((resolve, reject) => {
http
.get(
{
host: HOST,
port: PORT,
path: COV_DOWNLOAD_PATH,
},
(response) => {
response.pipe(zipFile);
zipFile.on('finish', () => {
zipFile.close();
resolve();
});
}
)
.on('error', (err) => {
fs.unlinkSync(zipFilename);
reject(err);
});
});
await /** @type {Promise<void>} */ (
new Promise((resolve, reject) => {
http
.get(
{
host: HOST,
port: PORT,
path: COV_DOWNLOAD_PATH,
},
(response) => {
response.pipe(zipFile);
zipFile.on('finish', () => {
zipFile.close();
resolve();
});
}
)
.on('error', (err) => {
fs.unlinkSync(zipFilename);
reject(err);
});
})
);
execOrDie(`unzip -o ${zipFilename} -d ${outDir}`);
}

Expand Down
6 changes: 3 additions & 3 deletions build-system/tasks/e2e/mocha-ci-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const {Base} = mocha.reporters;
*/
function ciReporter(runner, options) {
Base.call(this, runner, options);
this._mochaDotsReporter = new MochaDotsReporter(runner, options);
this._jsonReporter = new JsonReporter(runner, options);
rileyajones marked this conversation as resolved.
Show resolved Hide resolved
this._mochaDotsReporter = new MochaDotsReporter(runner);
this._jsonReporter = new JsonReporter(runner);
this._mochaJunitReporter = new MochaJUnitReporter(runner, options);
return this;
return /** @type {*} */ (this);
rileyajones marked this conversation as resolved.
Show resolved Hide resolved
}
ciReporter.prototype.__proto__ = Base.prototype;

Expand Down
4 changes: 2 additions & 2 deletions build-system/tasks/e2e/network-logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class NetworkLogger {

/**
* @param {PerformanceMethods} networkMethod
* @return {Promise<logging.Entry[]>}
* @return {Promise<*>}
*/
async getEntries_(networkMethod) {
const entries = await this.driver_
Expand All @@ -55,7 +55,7 @@ export class NetworkLogger {
/**
* Gets sent requests with an optional url to filter by.
* @param {string=} url
* @return {Promise<Array<logging.Entry>>}
* @return {Promise<Array<*>>}
*/
async getSentRequests(url) {
const entries = await this.getEntries_(
Expand Down
3 changes: 1 addition & 2 deletions build-system/tasks/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,7 @@ async function compileMinifiedJs(srcDir, srcFilename, destDir, options) {
* @param {string} destFilename
*/
function handleBundleError(err, continueOnError, destFilename) {
/** @type {Error|string} */
let message = err;
let message = err.toString();
if (err.stack) {
// Drop the node_modules call stack, which begins with ' at'.
message = err.stack.replace(/ at[^]*/, '').trim();
Expand Down
1 change: 1 addition & 0 deletions build-system/tasks/lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {ESLint} = require('eslint');
const {getFilesToCheck} = require('../common/utils');
const {lintGlobs} = require('../test-configs/config');

/** @type {ESLint.Options} */
const options = {
fix: argv.fix,
reportUnusedDisableDirectives: 'error',
Expand Down
1 change: 1 addition & 0 deletions build-system/tasks/performance-urls.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ async function performanceUrls() {
process.exitCode = 1;
return;
}
/** @type {string[]} */
const filepaths = jsonContent.handlers.flatMap((handler) =>
handler.urls
.filter((url) => url.startsWith(LOCAL_HOST_URL))
Expand Down
8 changes: 6 additions & 2 deletions build-system/tasks/report-test-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ async function postReport(type, action) {
'Content-Type': 'application/json',
},
});
const body = await response.text();
const body = /** @type {string} */ (await response.text());

log('Reported', cyan(`${type}/${action}`), 'to GitHub');
if (body.length > 0) {
Expand Down Expand Up @@ -211,7 +211,11 @@ async function reportAllExpectedTests() {
* Callback to the Karma.Server on('run_complete') event for simple test types.
* Optionally takes an object containing test results if they were run.
*
* @param {?Karma.TestResults} results
* @param {?{
* error: string|number,
* failed: string|number,
* success: string|number,
* }} results
*/
async function reportTestRunComplete(results) {
if (!results || results.error) {
Expand Down
1 change: 1 addition & 0 deletions build-system/tasks/runtime-test/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ function maybePrintArgvMessages() {
log(green('Running tests against unminified code.'));
}
Object.keys(argv).forEach((arg) => {
/** @type {string} */
const message = argvMessages[arg];
if (message) {
log(yellow(`--${arg}:`), green(message));
Expand Down
24 changes: 23 additions & 1 deletion build-system/tasks/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,30 @@ const {log} = require('../common/logging');
const {watchDebounceDelay} = require('./helpers');
const {watch} = require('chokidar');

/**
* @typedef {{
* name: string,
* port: string,
* root: string,
* host: string,
* debug?: boolean,
* silent?: boolean,
* https?: boolean,
* preferHttp1?: boolean,
* liveReload?: boolean,
* middleware?: function[],
* startedcallback?: function,
* serverInit?: function,
* fallback?: string,
* index: boolean | string | string[],
* }}
*/
let GulpConnectOptionsDef;

const argv = minimist(process.argv.slice(2), {string: ['rtv']});

const HOST = argv.host || '0.0.0.0';
const PORT = argv.port || 8000;
const PORT = argv.port || '8000';

// Used for logging.
let url = null;
Expand Down Expand Up @@ -105,6 +125,8 @@ async function startServer(
started = resolve;
});
setServeMode(modeOptions);

/** @type {GulpConnectOptionsDef} */
const options = {
name: 'AMP Dev Server',
root: process.cwd(),
Expand Down
2 changes: 1 addition & 1 deletion build-system/tasks/test-report-upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ async function sendCiKarmaReport(testType) {
'failed to report results of type',
cyan(testType),
': \n',
yellow(await response.text())
yellow(/** @type {string} */ (await response.text()))
);
}
}
Expand Down
8 changes: 4 additions & 4 deletions build-system/test-configs/jscodeshift/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ const command = (args = []) =>

/**
* @param {Array<string>} args
* @param {Object} opts
* @return {string}
* @param {?Object=} opts
* @return {!Object}
*/
const jscodeshift = (args = [], opts) => getOutput(command(args), opts);

/**
* @param {Array<string>} args
* @param {Object} opts
* @return {ChildProcess}
* @return {ReturnType<execScriptAsync>}
*/
const jscodeshiftAsync = (args = [], opts) =>
execScriptAsync(command(args), opts);
Expand All @@ -44,7 +44,7 @@ const stripColors = (str) => str.replace(/\x1B[[(?);]{0,2}(;?\d)*./g, '');

/**
* @param {string} line
* @return {Array<string>} [filename, report]
* @return {?Array<string>} [filename, report]
*/
function getJscodeshiftReport(line) {
const stripped = stripColors(line);
Expand Down
2 changes: 1 addition & 1 deletion build-system/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@
"server/typescript-compile.js",
"test-configs/dep-check-config.js",
"tasks/e2e/helper.js",
"tasks/get-zindex/test-1.js"
"tasks/get-zindex/*"
]
}