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

types: better types #1166

Merged
merged 14 commits into from
Mar 20, 2020
7 changes: 4 additions & 3 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ third_party/*
utils/browser/playwright-web.js
utils/doclint/check_public_api/test/
utils/testrunner/examples/
node6/*
node6-test/*
node6-testrunner/*
lib/
*.js
src/generated/*
src/chromium/protocol.ts
src/firefox/protocol.ts
src/webkit/protocol.ts
/types/*
/index.d.ts
utils/generate_types/overrides.d.ts
utils/generate_types/test/test.ts
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ lib/
playwright-*.tgz
/web.js
/web.js.map
/types/*
2 changes: 1 addition & 1 deletion .npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
# Injected files are included via lib/generated, see src/injected/README.md
lib/injected/
#types
!lib/**/*.d.ts
!types/*
!index.d.ts

# used for npm install scripts
Expand Down
2 changes: 1 addition & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3246,7 +3246,7 @@ ResourceType will be one of the following: `document`, `stylesheet`, `image`, `m
<!-- GEN:stop -->

#### response.body()
- returns: <Promise<[Buffer]>> Promise which resolves to a buffer with response body.
- returns: <[Promise]<[Buffer]>> Promise which resolves to a buffer with response body.

#### response.finished()
- returns: <[Promise]<?[Error]>> Waits for this response to finish, returns failure error if request failed.
Expand Down
14 changes: 6 additions & 8 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@
* limitations under the License.
*/

export * from './lib/api';
export const devices: typeof import('./lib/deviceDescriptors').DeviceDescriptors;
export const errors: { TimeoutError: typeof import('./lib/errors').TimeoutError };
export const chromium: import('./lib/server/chromium').Chromium;
export const firefox: import('./lib/server/firefox').Firefox;
export const webkit: import('./lib/server/webkit').WebKit;
export const selectors: import('./lib/api').Selectors;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about selectors, web, errors and devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is web public right now? How is it accessed? I did the others.

export type PlaywrightWeb = typeof import('./lib/web');
import * as types from './types/types';

export * from './types/types';
export const webkit: types.BrowserType<types.WebKitBrowser>;
export const chromium: types.BrowserType<types.ChromiumBrowser>;
export const firefox: types.BrowserType<types.FirefoxBrowser>;
13 changes: 10 additions & 3 deletions install-from-github.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

// This file is only run when someone installs via the github repo

const {execSync} = require('child_process');

try {
console.log('Building playwright...');
require('child_process').execSync('npm run build', {
execSync('npm run build', {
stdio: 'ignore'
});
} catch (e) {
Expand Down Expand Up @@ -85,8 +87,13 @@ const DOWNLOAD_PATHS = {
directories.add(path.join(__dirname, '.local-webkit'));
await Promise.all([...directories].map(directory => rmAsync(directory)));

try {
console.log('Generating types...');
execSync('npm run generate-types');
} catch (e) {
}

async function readdirAsync(dirpath) {
return fs.promises.readdir(dirpath).then(dirs => dirs.map(dir => path.join(dirpath, dir)));
}
})();

})();
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@
"test-infra": "node utils/doclint/check_public_api/test/test.js && node utils/doclint/preprocessor/test.js && node utils/testrunner/test/test.js",
"lint": "npm run eslint && npm run tsc && npm run doc && npm run test-infra",
"debug-test": "node --inspect-brk test/test.js",
"clean": "rimraf lib",
"clean": "rimraf lib && rimraf types",
"prepare": "node install-from-github.js",
"build": "node utils/runWebpack.js --mode='development' && tsc -p .",
"watch": "node utils/runWebpack.js --mode='development' --watch --silent | tsc -w -p .",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need something for watch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. I don't think it works right now in build though. It depends on the protocol.ts file to be generated, but the protocol generator requires the project to be built. I'll move it to a separate command that runs on bots, test, and before publishing.

"version": "node utils/sync_package_versions.js && npm run doc"
"version": "node utils/sync_package_versions.js && npm run doc",
"generate-types": "node utils/generate_types/"
},
"author": {
"name": "Microsoft Corporation"
Expand Down
20 changes: 20 additions & 0 deletions packages/playwright-chromium/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from 'playwright-core/types/types';

export * from 'playwright-core/types/types';
export const chromium: types.BrowserType<types.ChromiumBrowser>;
20 changes: 20 additions & 0 deletions packages/playwright-firefox/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from 'playwright-core/types/types';

export * from 'playwright-core/types/types';
export const firefox: types.BrowserType<types.FirefoxBrowser>;
20 changes: 20 additions & 0 deletions packages/playwright-webkit/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from 'playwright-core/types/types';

export * from 'playwright-core/types/types';
export const webkit: types.BrowserType<types.WebKitBrowser>;
23 changes: 22 additions & 1 deletion packages/playwright/index.d.ts
Original file line number Diff line number Diff line change
@@ -1 +1,22 @@
export * from "playwright-core";
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from 'playwright-core/types/types';

export * from 'playwright-core/types/types';
export const webkit: types.BrowserType<types.WebKitBrowser>;
export const chromium: types.BrowserType<types.ChromiumBrowser>;
export const firefox: types.BrowserType<types.FirefoxBrowser>;
1 change: 0 additions & 1 deletion packages/playwright/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,3 @@ try {
throw new Error('ERROR: Playwright did not download browsers');
}


4 changes: 2 additions & 2 deletions src/server/browserType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { Browser, ConnectOptions } from '../browser';
import { ConnectOptions } from '../browser';
import { BrowserContext } from '../browserContext';
import { BrowserServer } from './browserServer';

Expand All @@ -39,7 +39,7 @@ export type LaunchOptions = BrowserArgOptions & {
env?: {[key: string]: string} | undefined
};

export interface BrowserType {
export interface BrowserType<Browser> {
executablePath(): string;
name(): string;
launch(options?: LaunchOptions & { slowMo?: number }): Promise<Browser>;
Expand Down
2 changes: 1 addition & 1 deletion src/server/chromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { Events } from '../events';
import { ConnectionTransport } from '../transport';
import { BrowserContext } from '../browserContext';

export class Chromium implements BrowserType {
export class Chromium implements BrowserType<CRBrowser> {
private _executablePath: (string|undefined);

executablePath(): string {
Expand Down
2 changes: 1 addition & 1 deletion src/server/firefox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { launchProcess, waitForLine } from './processLauncher';

const mkdtempAsync = platform.promisify(fs.mkdtemp);

export class Firefox implements BrowserType {
export class Firefox implements BrowserType<FFBrowser> {
private _executablePath: (string|undefined);

executablePath(): string {
Expand Down
2 changes: 1 addition & 1 deletion src/server/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { BrowserServer } from './browserServer';
import { Events } from '../events';
import { BrowserContext } from '../browserContext';

export class WebKit implements BrowserType {
export class WebKit implements BrowserType<WKBrowser> {
private _executablePath: (string|undefined);

executablePath(): string {
Expand Down
2 changes: 1 addition & 1 deletion test/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ interface TestSetup<STATE> {
MAC: boolean;
LINUX: boolean;
WIN: boolean;
playwright: import('../src/server/browserType').BrowserType;
playwright: import('../src/server/browserType').BrowserType<import('../src/browser').Browser>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's migrate this whole file to public types in a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely!

selectors: import('../src/selectors').Selectors;
expect<T>(value: T): Expect<T>;
defaultBrowserOptions: import('../src/server/browserType').LaunchOptions;
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"rootDir": "./src",
"outDir": "./lib",
"strict": true,
"declaration": true
"declaration": false
},
"compileOnSave": true,
"include": ["src/**/*.ts"],
Expand Down
18 changes: 14 additions & 4 deletions utils/doclint/check_public_api/Documentation.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ Documentation.Class = class {
* @param {!Array<!Documentation.Member>} membersArray
* @param {?string=} extendsName
* @param {string=} comment
* @param {string[]=} templates
*/
constructor(name, membersArray, extendsName = null, comment = '') {
constructor(name, membersArray, extendsName = null, comment = '', templates = []) {
this.name = name;
this.membersArray = membersArray;
this.comment = comment;
this.extends = extendsName;
this.templates = templates;
this.index();
}

Expand Down Expand Up @@ -124,8 +126,12 @@ Documentation.Member = class {
* @param {string} name
* @param {?Documentation.Type} type
* @param {!Array<!Documentation.Member>} argsArray
* @param {string=} comment
* @param {string=} returnComment
* @param {boolean=} required
* @param {string[]=} templates
*/
constructor(kind, name, type, argsArray, comment = '', returnComment = '', required = true) {
constructor(kind, name, type, argsArray, comment = '', returnComment = '', required = true, templates = []) {
if (name === 'code') debugger;
this.kind = kind;
this.name = name;
Expand All @@ -134,6 +140,7 @@ Documentation.Member = class {
this.returnComment = returnComment;
this.argsArray = argsArray;
this.required = required;
this.templates = templates;
/** @type {!Map<string, !Documentation.Member>} */
this.args = new Map();
for (const arg of argsArray)
Expand All @@ -144,10 +151,13 @@ Documentation.Member = class {
* @param {string} name
* @param {!Array<!Documentation.Member>} argsArray
* @param {?Documentation.Type} returnType
* @param {string=} returnComment
* @param {string=} comment
* @param {string[]=} templates
* @return {!Documentation.Member}
*/
static createMethod(name, argsArray, returnType, returnComment, comment) {
return new Documentation.Member('method', name, returnType, argsArray, comment, returnComment);
static createMethod(name, argsArray, returnType, returnComment, comment, templates) {
return new Documentation.Member('method', name, returnType, argsArray, comment, returnComment, undefined, templates);
}

/**
Expand Down
12 changes: 8 additions & 4 deletions utils/doclint/check_public_api/JSBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function checkSources(sources, externalDependencies) {
visit(classesByName.get(parent));
};
visit(cls);
return new Documentation.Class(expandPrefix(cls.name), Array.from(membersMap.values()));
return new Documentation.Class(expandPrefix(cls.name), Array.from(membersMap.values()), undefined, cls.comment, cls.templates);
});
}

Expand Down Expand Up @@ -264,6 +264,7 @@ function checkSources(sources, externalDependencies) {
function serializeClass(className, symbol, node) {
/** @type {!Array<!Documentation.Member>} */
const members = classEvents.get(className) || [];
const templates = [];
for (const [name, member] of symbol.members || []) {
if (className === 'Error')
continue;
Expand All @@ -281,13 +282,15 @@ function checkSources(sources, externalDependencies) {
}
const memberType = checker.getTypeOfSymbolAtLocation(member, member.valueDeclaration);
const signature = signatureForType(memberType);
if (signature)
if (member.flags & ts.SymbolFlags.TypeParameter)
templates.push(name);
else if (signature)
members.push(serializeSignature(name, signature));
else
members.push(serializeProperty(name, memberType));
}

return new Documentation.Class(className, members);
return new Documentation.Class(className, members, undefined, undefined, templates);
}

/**
Expand All @@ -312,8 +315,9 @@ function checkSources(sources, externalDependencies) {
function serializeSignature(name, signature) {
const minArgumentCount = signature.minArgumentCount || 0;
const parameters = signature.parameters.map((s, index) => serializeSymbol(s, [], index < minArgumentCount));
const templates = signature.typeParameters ? signature.typeParameters.map(t => t.symbol.name) : [];
const returnType = serializeType(signature.getReturnType());
return Documentation.Member.createMethod(name, parameters, returnType.name !== 'void' ? returnType : null);
return Documentation.Member.createMethod(name, parameters, returnType.name !== 'void' ? returnType : null, undefined, undefined, templates);
}

/**
Expand Down
Loading