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: wider compat TS types and CI checks to ensure correct type defs #6855

Merged
merged 14 commits into from Feb 10, 2021
Merged
1 change: 1 addition & 0 deletions .eslintignore
Expand Up @@ -12,3 +12,4 @@ lib/
utils/doclint/generate_types/test/test.ts
vendor/
web-test-runner.config.mjs
test-ts-types/
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Expand Up @@ -45,6 +45,7 @@ jobs:
npm run lint
npm run generate-docs
npm run ensure-correct-devtools-protocol-revision
npm run test-types-file
jackfranklin marked this conversation as resolved.
Show resolved Hide resolved

- name: Run unit tests
env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-on-tag.yml
Expand Up @@ -3,7 +3,7 @@ name: publish-on-tag
on:
push:
tags:
- '*'
- '*'

jobs:
publish:
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
@@ -1,4 +1,6 @@
/node_modules/
test-ts-types/**/node_modules
test-ts-types/**/dist/
/test/output-chromium
/test/output-firefox
/test/test-user-data-dir*
Expand All @@ -18,3 +20,4 @@ test/coverage.json
temp/
puppeteer-core-*.tgz
new-docs/
puppeteer-*.tgz
3 changes: 2 additions & 1 deletion package.json
Expand Up @@ -33,9 +33,10 @@
"tsc-esm": "tsc -b src/tsconfig.esm.json",
"apply-next-version": "node utils/apply_next_version.js",
"test-install": "scripts/test-install.sh",
"generate-d-ts": "npm run tsc && api-extractor run --local --verbose && ts-node -s scripts/add-default-export-to-types.ts",
"generate-d-ts": "api-extractor run --local --verbose",
"generate-docs": "npm run generate-d-ts && api-documenter markdown -i temp -o new-docs",
"ensure-correct-devtools-protocol-revision": "ts-node -s scripts/ensure-correct-devtools-protocol-package",
"test-types-file": "ts-node -s scripts/test-ts-definition-files.ts",
"release": "node utils/remove_version_suffix.js && standard-version --commit-all"
},
"files": [
Expand Down
33 changes: 0 additions & 33 deletions scripts/add-default-export-to-types.ts

This file was deleted.

188 changes: 188 additions & 0 deletions scripts/test-ts-definition-files.ts
@@ -0,0 +1,188 @@
import { spawnSync } from 'child_process';
import { version } from '../package.json';
import path from 'path';
const PROJECT_FOLDERS_ROOT = 'test-ts-types';
const EXPECTED_ERRORS = new Map<string, string[]>([
[
'ts-esm-import-esm-output',
[
"bad.ts(6,35): error TS2551: Property 'launh' does not exist on type",
"bad.ts(8,29): error TS2551: Property 'devics' does not exist on type",
'bad.ts(12,39): error TS2554: Expected 0 arguments, but got 1.',
],
],
[
'ts-esm-import-cjs-output',
[
"bad.ts(6,35): error TS2551: Property 'launh' does not exist on type",
"bad.ts(8,29): error TS2551: Property 'devics' does not exist on type",
'bad.ts(12,39): error TS2554: Expected 0 arguments, but got 1.',
],
],
[
'ts-cjs-import-cjs-output',
[
"bad.ts(5,35): error TS2551: Property 'launh' does not exist on type",
"bad.ts(7,29): error TS2551: Property 'devics' does not exist on type",
'bad.ts(11,39): error TS2554: Expected 0 arguments, but got 1.',
],
],
[
'js-esm-import-cjs-output',
[
"bad.js(5,35): error TS2551: Property 'launh' does not exist on type",
"bad.js(7,29): error TS2551: Property 'devics' does not exist on type",
'bad.js(11,39): error TS2554: Expected 0 arguments, but got 1.',
"bad.js(15,9): error TS2322: Type 'ElementHandle<Element> | null' is not assignable to type 'ElementHandle<HTMLElement>'",
],
],
[
'js-cjs-import-esm-output',
[
"bad.js(5,35): error TS2551: Property 'launh' does not exist on type",
"bad.js(7,29): error TS2551: Property 'devics' does not exist on type",
'bad.js(11,39): error TS2554: Expected 0 arguments, but got 1.',
"bad.js(15,9): error TS2322: Type 'ElementHandle<Element> | null' is not assignable to type 'ElementHandle<HTMLElement>'",
],
],
[
'js-esm-import-esm-output',
[
"bad.js(5,35): error TS2551: Property 'launh' does not exist on type",
"bad.js(7,29): error TS2551: Property 'devics' does not exist on type",
'bad.js(11,39): error TS2554: Expected 0 arguments, but got 1.',
"bad.js(15,9): error TS2322: Type 'ElementHandle<Element> | null' is not assignable to type 'ElementHandle<HTMLElement>'",
],
],
[
'js-cjs-import-cjs-output',
[
"bad.js(5,35): error TS2551: Property 'launh' does not exist on type",
"bad.js(7,29): error TS2551: Property 'devics' does not exist on type",
'bad.js(11,39): error TS2554: Expected 0 arguments, but got 1.',
"bad.js(15,9): error TS2322: Type 'ElementHandle<Element> | null' is not assignable to type 'ElementHandle<HTMLElement>'",
],
],
]);
const PROJECT_FOLDERS = [...EXPECTED_ERRORS.keys()];

function packPuppeteer() {
console.log('Packing Puppeteer');
const result = spawnSync('npm', ['pack'], {
encoding: 'utf-8',
});

if (result.status !== 0) {
console.log('Failed to pack Puppeteer', result.stderr);
process.exit(1);
}

return `puppeteer-${version}.tgz`;
}

const tar = packPuppeteer();
const tarPath = path.join(process.cwd(), tar);

function cdIntoProjectAndRunCommand(projectLocation: string, command: string) {
return spawnSync(
'sh',
['-c', [`cd "${projectLocation}"`, command].join('; ')],
jackfranklin marked this conversation as resolved.
Show resolved Hide resolved
{ encoding: 'utf-8', stdio: 'pipe' }
);
}

function compileAndCatchErrors(projectLocation) {
const { status, stdout, stderr } = cdIntoProjectAndRunCommand(
projectLocation,
'npm run compile'
);
const tsErrorMesssage = stdout.split('\n');

if (status === 0) {
console.error(
`Running tsc on ${projectLocation} succeeded without triggering the expected errors.`
);
console.log(stdout, stderr);
process.exit(1);
}

return {
tsErrorMesssage,
};
}

function testProject(folder: string) {
console.log('\nTesting:', folder);
const projectLocation = path.join(
process.cwd(),
PROJECT_FOLDERS_ROOT,
folder
);

const tarLocation = path.relative(projectLocation, tarPath);
console.log('===> Installing Puppeteer from tar file');
const { status, stderr, stdout } = cdIntoProjectAndRunCommand(
projectLocation,
`PUPPETEER_SKIP_DOWNLOAD=1 npm install --no-package-lock "${tarLocation}"`
);

if (status > 0) {
console.error(
'Installing the tar file unexpectedly failed',
stdout,
stderr
);
process.exit(status);
}
console.log('===> Running compile to ensure expected errors only.');
const result = compileAndCatchErrors(projectLocation);
const expectedErrors = EXPECTED_ERRORS.get(folder) || [];
if (
result.tsErrorMesssage.find(
(line) => line.includes('good.ts') || line.includes('good.js')
)
) {
console.error(
`Error for ${projectLocation} contained unexpected failures in good.ts/good.js:\n${result.tsErrorMesssage.join(
'\n'
)}`
);
process.exit(1);
}
const errorsInTsMessage = result.tsErrorMesssage.filter(
(line) => line.includes('bad.ts') || line.includes('bad.js')
);
const expectedErrorsThatHaveOccurred = new Set<string>();
const unexpectedErrors = errorsInTsMessage.filter((message) => {
const isExpected = expectedErrors.some((expectedError) => {
const isExpected = message.startsWith(expectedError);
if (isExpected) {
expectedErrorsThatHaveOccurred.add(expectedError);
}
return isExpected;
});
return !isExpected;
});

if (unexpectedErrors.length) {
console.error(
`${projectLocation} had unexpected TS errors: ${unexpectedErrors.join(
'\n'
)}`
);
process.exit(1);
}
expectedErrors.forEach((expected) => {
if (!expectedErrorsThatHaveOccurred.has(expected)) {
console.error(
`${projectLocation} expected error that was not thrown: ${expected}`
);
process.exit(1);
}
});
console.log('===> ✅ Type-checked correctly.');
}

PROJECT_FOLDERS.forEach((folder) => {
testProject(folder);
});
81 changes: 81 additions & 0 deletions src/api-docs-entry.ts
Expand Up @@ -14,6 +14,16 @@
* limitations under the License.
*/

import { LaunchOptions, ChromeArgOptions } from './node/LaunchOptions.js';
import { BrowserOptions } from './common/BrowserConnector.js';
import { Product } from './common/Product.js';
import { Browser } from './common/Browser.js';
import { ConnectOptions } from './common/Puppeteer.js';
import { DevicesMap } from './common/DeviceDescriptors.js';
import { PuppeteerErrors } from './common/Errors.js';
import { PredefinedNetworkConditions } from './common/NetworkConditions.js';
import { CustomQueryHandler } from './common/QueryHandler.js';

/*
* This file re-exports any APIs that we want to have documentation generated
* for. It is used by API Extractor to determine what parts of the system to
Expand Down Expand Up @@ -62,4 +72,75 @@ export * from './common/PDFOptions.js';
export * from './common/TimeoutSettings.js';
export * from './common/LifecycleWatcher.js';
export * from './common/QueryHandler.js';
export * from './common/NetworkConditions.js';
export * from 'devtools-protocol/types/protocol';

/*
* We maintain a namespace that emulates the API of the Puppeteer instance you
* get when you `import puppeteer from 'puppeteer'.
*
* We do this as a namespace because export = PuppeteerDefault where
* PuppeteerDefault is a namespace seems to make sure that the types work in
* both ESM and CJS contexts.
*
* This namespace must be kept in sync with the public API offered by the
* PuppeteerNode class.
*/

/**
* @public
* {@inheritDoc PuppeteerNode.launch}
*/
export declare function launch(
options?: LaunchOptions &
ChromeArgOptions &
BrowserOptions & {
product?: Product;
extraPrefsFirefox?: Record<string, unknown>;
}
): Promise<Browser>;

/**
* @public
* {@inheritDoc PuppeteerNode.connect}
*/
export declare function connect(options: ConnectOptions): Promise<Browser>;

/**
* @public
* {@inheritDoc Puppeteer.devices}
*/
export let devices: DevicesMap;
/**
* @public
*/
export let errors: PuppeteerErrors;
/**
* @public
*/
export let networkConditions: PredefinedNetworkConditions;

/**
* @public
* {@inheritDoc Puppeteer.registerCustomQueryHandler}
*/
export declare function registerCustomQueryHandler(
name: string,
queryHandler: CustomQueryHandler
): void;

/**
* @public
* {@inheritDoc Puppeteer.unregisterCustomQueryHandler}
*/
export declare function unregisterCustomQueryHandler(name: string): void;
/**
* @public
* {@inheritDoc Puppeteer.customQueryHandlerNames}
*/
export declare function customQueryHandlerNames(): string[];
/**
* @public
* {@inheritDoc Puppeteer.clearCustomQueryHandlers}
*/
export declare function clearCustomQueryHandlers(): void;
6 changes: 6 additions & 0 deletions src/common/NetworkConditions.ts
Expand Up @@ -16,8 +16,14 @@

import { NetworkConditions } from './NetworkManager.js';

/**
* @public
*/
export type PredefinedNetworkConditions = { [name: string]: NetworkConditions };

/**
* @public
*/
export const networkConditions: PredefinedNetworkConditions = {
'Slow 3G': {
download: ((500 * 1000) / 8) * 0.8,
Expand Down
18 changes: 18 additions & 0 deletions test-ts-types/js-cjs-import-cjs-output/bad.js
@@ -0,0 +1,18 @@
const puppeteer = require('puppeteer');

async function run() {
// Typo in "launch"
const browser = await puppeteer.launh();
// Typo: "devices"
const devices = puppeteer.devics;
console.log(devices);
const browser2 = await puppeteer.launch();
// 'foo' is invalid argument
const page = await browser2.newPage('foo');
/**
* @type {puppeteer.ElementHandle<HTMLElement>}
*/
const div = await page.$('div');
console.log('got a div!', div);
}
run();