Skip to content

Commit

Permalink
feat: support node 18
Browse files Browse the repository at this point in the history
  • Loading branch information
jrandolf committed Jun 1, 2022
1 parent 3f4451e commit 90b6a47
Show file tree
Hide file tree
Showing 19 changed files with 38 additions and 76 deletions.
3 changes: 3 additions & 0 deletions .eslintrc.js
Expand Up @@ -124,7 +124,10 @@ module.exports = {
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
],
plugins: ['eslint-plugin-tsdoc'],
rules: {
// Error if comments do not adhere to `tsdoc`.
'tsdoc/syntax': 2,
'no-unused-vars': 0,
'@typescript-eslint/no-unused-vars': [
'error',
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Expand Up @@ -23,7 +23,7 @@ jobs:
matrix:
# Include all major maintenance + active LTS + current Node.js versions.
# https://github.com/nodejs/Release#release-schedule
node: [14, 16]
node: [14, 16, 18]
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -120,6 +120,7 @@
"eslint-plugin-import": "2.26.0",
"eslint-plugin-mocha": "10.0.5",
"eslint-plugin-prettier": "4.0.0",
"eslint-plugin-tsdoc": "0.2.16",
"eslint-plugin-unicorn": "42.0.0",
"esprima": "4.0.1",
"expect": "25.2.7",
Expand Down
2 changes: 1 addition & 1 deletion scripts/ensure-correct-devtools-protocol-package.ts
Expand Up @@ -27,7 +27,7 @@
* not true that each Chromium revision will have an exact matching revision
* version of devtools-protocol. To ensure we're using a devtools-protocol that
* is aligned with our revision, we want to find the largest package number
* that's <= the revision that Puppeteer is using.
* that's \<= the revision that Puppeteer is using.
*
* This script uses npm's `view` function to list all versions in a range and
* find the one closest to our Chromium revision.
Expand Down
2 changes: 1 addition & 1 deletion src/.eslintrc.js
Expand Up @@ -6,7 +6,7 @@ module.exports = {
* All available rules: http://eslint.org/docs/rules/
*
* Rules take the following form:
* "rule-name", [severity, { opts }]
* "rule-name", [severity, \{ opts \}]
* Severity: 2 == error, 1 == warning, 0 == off.
*/
rules: {
Expand Down
11 changes: 0 additions & 11 deletions src/common/Connection.ts
Expand Up @@ -363,12 +363,6 @@ export class CDPSession extends EventEmitter {
}
}

/**
* @param {!Error} error
* @param {string} method
* @param {{error: {message: string, data: any}}} object
* @returns {!Error}
*/
function createProtocolError(
error: ProtocolError,
method: string,
Expand All @@ -379,11 +373,6 @@ function createProtocolError(
return rewriteError(error, message, object.error.message);
}

/**
* @param {!Error} error
* @param {string} message
* @returns {!Error}
*/
function rewriteError(
error: ProtocolError,
message: string,
Expand Down
7 changes: 2 additions & 5 deletions src/common/DOMWorld.ts
Expand Up @@ -335,7 +335,7 @@ export class DOMWorld {
'Cannot pass a filepath to addScriptTag in the browser environment.'
);
}
const fs = await helper.importFSModule();
const fs = await import('fs');
let contents = await fs.promises.readFile(path, 'utf8');
contents += '//# sourceURL=' + path.replace(/\n/g, '');
const context = await this.executionContext();
Expand Down Expand Up @@ -442,7 +442,7 @@ export class DOMWorld {
'Cannot pass a filepath to addStyleTag in the browser environment.'
);
}
const fs = await helper.importFSModule();
const fs = await import('fs');
let contents = await fs.promises.readFile(path, 'utf8');
contents += '/*# sourceURL=' + path.replace(/\n/g, '') + '*/';
const context = await this.executionContext();
Expand Down Expand Up @@ -985,9 +985,6 @@ async function waitForPredicatePageFunction(
return await pollInterval(polling);
}

/**
* @returns {!Promise<*>}
*/
async function pollMutation(): Promise<unknown> {
const success = predicateAcceptsContextElement
? await predicate(root, ...args)
Expand Down
2 changes: 1 addition & 1 deletion src/common/Events.ts
Expand Up @@ -24,7 +24,7 @@
* add a new Page event, you should update the PageEmittedEvents enum in
* src/common/Page.ts.
*
* Chat to @jackfranklin if you're unsure.
* Chat to \@jackfranklin if you're unsure.
*/

export const Events = {
Expand Down
2 changes: 1 addition & 1 deletion src/common/JSHandle.ts
Expand Up @@ -829,7 +829,7 @@ export class ElementHandle<
avoid paying the cost unnecessarily.
*/
const path = await import('path');
const fs = await helper.importFSModule();
const fs = await import('fs');
// Locate all files and confirm that they exist.
const files = await Promise.all(
filePaths.map(async (filePath) => {
Expand Down
5 changes: 0 additions & 5 deletions src/common/LifecycleWatcher.ts
Expand Up @@ -236,11 +236,6 @@ export class LifecycleWatcher {
if (this._swapped || this._newDocumentNavigation)
this._newDocumentNavigationCompleteCallback();

/**
* @param {!Frame} frame
* @param {!Array<string>} expectedLifecycle
* @returns {boolean}
*/
function checkLifecycle(
frame: Frame,
expectedLifecycle: ProtocolLifeCycleEvent[]
Expand Down
8 changes: 4 additions & 4 deletions src/common/Page.ts
Expand Up @@ -2830,8 +2830,8 @@ export class Page extends EventEmitter {
'Screenshots can only be written to a file path in a Node environment.'
);
}
const fs = await helper.importFSModule();
await fs.promises.writeFile(options.path, buffer);
const fs = (await import('fs')).promises;
await fs.writeFile(options.path, buffer);
}
return buffer;

Expand Down Expand Up @@ -3416,9 +3416,9 @@ function convertPrintParameterToInches(
let pixels;
if (helper.isNumber(parameter)) {
// Treat numbers as pixel values to be aligned with phantom's paperSize.
pixels = /** @type {number} */ parameter;
pixels = parameter;
} else if (helper.isString(parameter)) {
const text = /** @type {string} */ parameter;
const text = parameter;
let unit = text.substring(text.length - 2).toLowerCase();
let valueText = '';
if (unit in unitToPixels) {
Expand Down
2 changes: 1 addition & 1 deletion src/common/assert.ts
Expand Up @@ -16,7 +16,7 @@

/**
* Asserts that the given value is truthy.
* @param value
* @param value - some conditional statement
* @param message - the error message to throw if the value is not truthy.
*/
export const assert: (value: unknown, message?: string) => asserts value = (
Expand Down
31 changes: 3 additions & 28 deletions src/common/helper.ts
Expand Up @@ -322,18 +322,18 @@ async function getReadableAsBuffer(
throw new Error('Cannot write to a path outside of Node.js environment.');
}

const fs = isNode ? await importFSModule() : null;
const fs = isNode ? (await import('fs')).promises : null;

let fileHandle: import('fs').promises.FileHandle | undefined;

if (path && fs) {
fileHandle = await fs.promises.open(path, 'w');
fileHandle = await fs.open(path, 'w');
}
const buffers = [];
for await (const chunk of readable) {
buffers.push(chunk);
if (fileHandle && fs) {
await fs.promises.writeFile(fileHandle, chunk);
await fs.writeFile(fileHandle, chunk);
}
}

Expand Down Expand Up @@ -376,30 +376,6 @@ async function getReadableFromProtocolStream(
});
}

/**
* Loads the Node fs promises API. Needed because on Node 10.17 and below,
* fs.promises is experimental, and therefore not marked as enumerable. That
* means when TypeScript compiles an `import('fs')`, its helper doesn't spot the
* promises declaration and therefore on Node <10.17 you get an error as
* fs.promises is undefined in compiled TypeScript land.
*
* See https://github.com/puppeteer/puppeteer/issues/6548 for more details.
*
* Once Node 10 is no longer supported (April 2021) we can remove this and use
* `(await import('fs')).promises`.
*/
async function importFSModule(): Promise<typeof import('fs')> {
if (!isNode) {
throw new Error('Cannot load the fs module API outside of Node.');
}

const fs = await import('fs');
if (fs.promises) {
return fs;
}
return fs.default;
}

export const helper = {
evaluationString,
pageBindingInitString,
Expand All @@ -413,7 +389,6 @@ export const helper = {
waitForEvent,
isString,
isNumber,
importFSModule,
addEventListener,
removeEventListeners,
valueFromRemoteObject,
Expand Down
3 changes: 0 additions & 3 deletions src/node/BrowserRunner.ts
Expand Up @@ -277,9 +277,6 @@ function waitForWSEndpoint(
];
const timeoutId = timeout ? setTimeout(onTimeout, timeout) : 0;

/**
* @param {!Error=} error
*/
function onClose(error?: Error): void {
cleanup();
reject(
Expand Down
4 changes: 2 additions & 2 deletions src/node/Launcher.ts
Expand Up @@ -687,8 +687,8 @@ class FirefoxLauncher implements ProductLauncher {
* able to restore the original values of preferences a backup of prefs.js
* will be created.
*
* @param prefs List of preferences to add.
* @param profilePath Firefox profile to write the preferences to.
* @param prefs - List of preferences to add.
* @param profilePath - Firefox profile to write the preferences to.
*/
async writePreferences(
prefs: { [x: string]: unknown },
Expand Down
14 changes: 13 additions & 1 deletion src/node/NodeWebSocketTransport.ts
Expand Up @@ -16,9 +16,21 @@
import NodeWebSocket from 'ws';
import { ConnectionTransport } from '../common/ConnectionTransport.js';
import { packageVersion } from '../generated/version.js';
import { promises as dns } from 'dns';

export class NodeWebSocketTransport implements ConnectionTransport {
static create(url: string): Promise<NodeWebSocketTransport> {
static async create(urlString: string): Promise<NodeWebSocketTransport> {
// TODO(jrandolf): Starting in Node 17, IPv6 is favoured over IPv4 due to a change
// in a default option:
// - https://github.com/nodejs/node/issues/40537,
// Due to this, we parse and resolve the hostname manually with the previous
// behavior according to:
// - https://nodejs.org/api/dns.html#dnslookuphostname-options-callback
// because of https://bugzilla.mozilla.org/show_bug.cgi?id=1769994.
const url = new URL(urlString);
const { address } = await dns.lookup(url.hostname, { verbatim: false });
url.hostname = address;

return new Promise((resolve, reject) => {
const ws = new NodeWebSocket(url, [], {
followRedirects: true,
Expand Down
6 changes: 3 additions & 3 deletions test/NetworkManager.spec.ts
Expand Up @@ -480,14 +480,14 @@ describeChromeOnly('NetworkManager', () => {
* This sequence was taken from an actual CDP session produced by the following
* test script:
*
* const browser = await puppeteer.launch({ headless: false });
* const browser = await puppeteer.launch(\{ headless: false \});
* const page = await browser.newPage();
* await page.setCacheEnabled(false);
*
* await page.setRequestInterception(true)
* page.on('request', (interceptedRequest) => {
* page.on('request', (interceptedRequest) =\> \{
* interceptedRequest.continue();
* });
* \});
*
* await page.goto('https://www.google.com');
* await browser.close();
Expand Down
3 changes: 0 additions & 3 deletions test/oopif.spec.ts
Expand Up @@ -419,9 +419,6 @@ describeChromeOnly('OOPIF', function () {
});
});

/**
* @param {!BrowserContext} context
*/
function oopifs(context) {
return context
.targets()
Expand Down
6 changes: 1 addition & 5 deletions test/requestinterception.spec.ts
Expand Up @@ -806,11 +806,7 @@ describe('request interception', function () {
});
});

/**
* @param {string} path
* @returns {string}
*/
function pathToFileURL(path) {
function pathToFileURL(path: string): string {
let pathName = path.replace(/\\/g, '/');
// Windows drive letter must be prefixed with a slash.
if (!pathName.startsWith('/')) pathName = '/' + pathName;
Expand Down

0 comments on commit 90b6a47

Please sign in to comment.