From 3e4c8c9d0d8747e4ce4b6712b6681a8c4feb68c1 Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Thu, 16 Apr 2020 14:59:28 +0100 Subject: [PATCH] chore(typescript): migrate src/Dialog (#5639) This PR changes `src/Dialog.js` to `src/Dialog.ts` and rewrites accordingly. Most of the changes are straight forward; the only interesting one from a TS point of view is the `DialogType` enum. I expose it again as `Dialog.Type` to avoid a breaking change. This PR also exposed some bugs with our ESLint TypeScript settings and applying the overrides, so I fixed those too. I also updated our DocLint tool to work on TS source files over JS lib files if they exist. This is the minimal change to keep the existing doc system working as we're working on moving away from this system longer term. --- .eslintrc.js | 11 ++-- src/{Dialog.js => Dialog.ts} | 59 +++++++++------------ utils/doclint/check_public_api/JSBuilder.js | 19 ++++++- utils/doclint/cli.js | 7 ++- 4 files changed, 57 insertions(+), 39 deletions(-) rename src/{Dialog.js => Dialog.ts} (66%) diff --git a/.eslintrc.js b/.eslintrc.js index a0bf98660eb0e..85305e5027cc1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -102,12 +102,17 @@ module.exports = { }, "overrides": [ { - // apply TypeScript linting to the TS files in src/ - "files": ["src/*.ts"], + "files": ["*.ts"], "extends": [ 'plugin:@typescript-eslint/eslint-recommended', 'plugin:@typescript-eslint/recommended', - ] + ], + "rules": { + "no-unused-vars": 0, + "@typescript-eslint/no-unused-vars": 2, + "semi": 0, + "@typescript-eslint/semi": 2 + } } ] }; diff --git a/src/Dialog.js b/src/Dialog.ts similarity index 66% rename from src/Dialog.js rename to src/Dialog.ts index cbdc271916937..8e0e0c0be45db 100644 --- a/src/Dialog.js +++ b/src/Dialog.ts @@ -14,48 +14,46 @@ * limitations under the License. */ -const {assert} = require('./helper'); +import helpers = require('./helper'); + +const {assert} = helpers; + +enum DialogType { + Alert = 'alert', + BeforeUnload = 'beforeunload', + Confirm = 'confirm', + Prompt = 'prompt' +} class Dialog { - /** - * @param {!Puppeteer.CDPSession} client - * @param {string} type - * @param {string} message - * @param {(string|undefined)} defaultValue - */ - constructor(client, type, message, defaultValue = '') { + static Type = DialogType; + + private _client: Puppeteer.CDPSession; + private _type: DialogType; + private _message: string; + private _defaultValue: string; + private _handled = false; + + constructor(client: Puppeteer.CDPSession, type: DialogType, message: string, defaultValue = '') { this._client = client; this._type = type; this._message = message; - this._handled = false; this._defaultValue = defaultValue; } - /** - * @return {string} - */ - type() { + type(): DialogType { return this._type; } - /** - * @return {string} - */ - message() { + message(): string { return this._message; } - /** - * @return {string} - */ - defaultValue() { + defaultValue(): string { return this._defaultValue; } - /** - * @param {string=} promptText - */ - async accept(promptText) { + async accept(promptText?: string): Promise { assert(!this._handled, 'Cannot accept dialog which is already handled!'); this._handled = true; await this._client.send('Page.handleJavaScriptDialog', { @@ -64,7 +62,7 @@ class Dialog { }); } - async dismiss() { + async dismiss(): Promise { assert(!this._handled, 'Cannot dismiss dialog which is already handled!'); this._handled = true; await this._client.send('Page.handleJavaScriptDialog', { @@ -73,11 +71,4 @@ class Dialog { } } -Dialog.Type = { - Alert: 'alert', - BeforeUnload: 'beforeunload', - Confirm: 'confirm', - Prompt: 'prompt' -}; - -module.exports = {Dialog}; +export = {Dialog}; diff --git a/utils/doclint/check_public_api/JSBuilder.js b/utils/doclint/check_public_api/JSBuilder.js index 065dc818c8803..c442b0529b2f9 100644 --- a/utils/doclint/check_public_api/JSBuilder.js +++ b/utils/doclint/check_public_api/JSBuilder.js @@ -3,6 +3,7 @@ const path = require('path'); const Documentation = require('./Documentation'); module.exports = checkSources; + /** * @param {!Array} sources */ @@ -30,7 +31,23 @@ function checkSources(sources) { const classes = []; /** @type {!Map} */ const inheritance = new Map(); - sourceFiles.filter(x => !x.fileName.includes('node_modules')).map(x => visit(x)); + + const sourceFilesNoNodeModules = sourceFiles.filter(x => !x.fileName.includes('node_modules')); + const sourceFileNamesSet = new Set(sourceFilesNoNodeModules.map(x => x.fileName)); + sourceFilesNoNodeModules.map(x => { + if (x.fileName.includes('/lib/')) { + const potentialTSSource = x.fileName.replace('lib', 'src').replace('.js', '.ts'); + if (sourceFileNamesSet.has(potentialTSSource)) { + /* Not going to visit this file because we have the TypeScript src code + * which we'll use instead. + */ + return; + } + } + + visit(x); + }); + const errors = []; const documentation = new Documentation(recreateClassesWithInheritance(classes, inheritance)); diff --git a/utils/doclint/cli.js b/utils/doclint/cli.js index 76156bb6cfbf0..0708c8474c8c3 100755 --- a/utils/doclint/cli.js +++ b/utils/doclint/cli.js @@ -49,8 +49,13 @@ async function run() { const browser = await puppeteer.launch(); const page = await browser.newPage(); const checkPublicAPI = require('./check_public_api'); + const tsSources = await Source.readdir(path.join(PROJECT_DIR, 'src'), 'ts'); + + const tsSourcesNoDefinitions = tsSources.filter(source => !source.filePath().endsWith('.d.ts')); + const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib')); - messages.push(...await checkPublicAPI(page, mdSources, jsSources)); + const allSrcCode = [...jsSources, ...tsSourcesNoDefinitions]; + messages.push(...await checkPublicAPI(page, mdSources, allSrcCode)); await browser.close();