Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jackfranklin committed Apr 16, 2020
1 parent a9f6a26 commit 3e4c8c9
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 39 deletions.
11 changes: 8 additions & 3 deletions .eslintrc.js
Expand Up @@ -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
}
}
]
};
59 changes: 25 additions & 34 deletions src/Dialog.js → src/Dialog.ts
Expand Up @@ -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<void> {
assert(!this._handled, 'Cannot accept dialog which is already handled!');
this._handled = true;
await this._client.send('Page.handleJavaScriptDialog', {
Expand All @@ -64,7 +62,7 @@ class Dialog {
});
}

async dismiss() {
async dismiss(): Promise<void> {
assert(!this._handled, 'Cannot dismiss dialog which is already handled!');
this._handled = true;
await this._client.send('Page.handleJavaScriptDialog', {
Expand All @@ -73,11 +71,4 @@ class Dialog {
}
}

Dialog.Type = {
Alert: 'alert',
BeforeUnload: 'beforeunload',
Confirm: 'confirm',
Prompt: 'prompt'
};

module.exports = {Dialog};
export = {Dialog};
19 changes: 18 additions & 1 deletion utils/doclint/check_public_api/JSBuilder.js
Expand Up @@ -3,6 +3,7 @@ const path = require('path');
const Documentation = require('./Documentation');
module.exports = checkSources;


/**
* @param {!Array<!import('../Source')>} sources
*/
Expand Down Expand Up @@ -30,7 +31,23 @@ function checkSources(sources) {
const classes = [];
/** @type {!Map<string, string>} */
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));

Expand Down
7 changes: 6 additions & 1 deletion utils/doclint/cli.js
Expand Up @@ -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();

Expand Down

0 comments on commit 3e4c8c9

Please sign in to comment.