From caa9a1cafa6510acc2344cd1ae67fce4c5ee4df5 Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Mon, 28 Sep 2020 10:35:35 +0100 Subject: [PATCH] chore(agnostic): Remove use of util.promisify (#6446) In `src/common` we now use `fs.promises.X` which we can dynamically `import`. In a browser environment this code will never run because it's gated on `isNode` (in a future PR we will add tree-shaking to the bundle step such that this code is eliminated). By using `import`, we ensure TypeScript still can track types and give good type information. In `src/node` we continue to use `util.promisify` but that's not a concern as that code explicitly is never run in the browser. --- package.json | 2 +- scripts/tsconfig.json | 6 ++++++ src/common/DOMWorld.ts | 16 ++++------------ src/common/JSHandle.ts | 17 +++++++++-------- src/common/Page.ts | 13 ++++++++----- src/common/helper.ts | 15 ++++++--------- test/tsconfig.test.json | 3 +++ tsconfig.base.json | 1 + 8 files changed, 38 insertions(+), 35 deletions(-) create mode 100644 scripts/tsconfig.json diff --git a/package.json b/package.json index 8c025186ed5..43e7be3a8f2 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "generate-docs": "npm run tsc && api-extractor run --local --verbose && api-documenter markdown -i temp -o new-docs", "ensure-new-docs-up-to-date": "npm run generate-docs && git status && exit `git status --porcelain | head -255 | wc -l`", "generate-dependency-graph": "echo 'Requires graphviz installed locally!' && depcruise --exclude 'api.ts' --do-not-follow '^node_modules' --output-type dot src/index.ts | dot -T png > dependency-chart.png", - "ensure-correct-devtools-protocol-revision": "ts-node scripts/ensure-correct-devtools-protocol-package" + "ensure-correct-devtools-protocol-revision": "ts-node -s scripts/ensure-correct-devtools-protocol-package" }, "files": [ "lib/", diff --git a/scripts/tsconfig.json b/scripts/tsconfig.json new file mode 100644 index 00000000000..c8d842173c6 --- /dev/null +++ b/scripts/tsconfig.json @@ -0,0 +1,6 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "module": "CommonJS" + } +} diff --git a/src/common/DOMWorld.ts b/src/common/DOMWorld.ts index bb859dc0e3c..d078116aede 100644 --- a/src/common/DOMWorld.ts +++ b/src/common/DOMWorld.ts @@ -275,12 +275,8 @@ export class DOMWorld { 'Cannot pass a filepath to addScriptTag in the browser environment.' ); } - // eslint-disable-next-line @typescript-eslint/no-var-requires - const fs = require('fs'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { promisify } = require('util'); - const readFileAsync = promisify(fs.readFile); - let contents = await readFileAsync(path, 'utf8'); + const fs = await import('fs'); + let contents = await fs.promises.readFile(path, 'utf8'); contents += '//# sourceURL=' + path.replace(/\n/g, ''); const context = await this.executionContext(); return ( @@ -361,12 +357,8 @@ export class DOMWorld { 'Cannot pass a filepath to addStyleTag in the browser environment.' ); } - // eslint-disable-next-line @typescript-eslint/no-var-requires - const fs = require('fs'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { promisify } = require('util'); - const readFileAsync = promisify(fs.readFile); - let contents = await readFileAsync(path, 'utf8'); + const fs = await import('fs'); + let contents = await fs.promises.readFile(path, 'utf8'); contents += '/*# sourceURL=' + path.replace(/\n/g, '') + '*/'; const context = await this.executionContext(); return ( diff --git a/src/common/JSHandle.ts b/src/common/JSHandle.ts index afb6fc7eafb..ebf31517a54 100644 --- a/src/common/JSHandle.ts +++ b/src/common/JSHandle.ts @@ -31,6 +31,7 @@ import { WrapElementHandle, UnwrapPromiseLike, } from './EvalTypes.js'; +import { isNode } from '../environment.js'; export interface BoxModel { content: Array<{ x: number; y: number }>; @@ -556,22 +557,22 @@ export class ElementHandle< 'Multiple file uploads only work with ' ); + if (!isNode) { + throw new Error( + `JSHandle#uploadFile can only be used in Node environments.` + ); + } // This import is only needed for `uploadFile`, so keep it scoped here to avoid paying // the cost unnecessarily. + const path = await import('path'); // eslint-disable-next-line @typescript-eslint/no-var-requires - const path = require('path'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const fs = require('fs'); - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { promisify } = require('util'); - const access = promisify(fs.access); - + const fs = await import('fs'); // Locate all files and confirm that they exist. const files = await Promise.all( filePaths.map(async (filePath) => { const resolvedPath: string = path.resolve(filePath); try { - await access(resolvedPath, fs.constants.R_OK); + await fs.promises.access(resolvedPath, fs.constants.R_OK); } catch (error) { if (error.code === 'ENOENT') throw new Error(`${filePath} does not exist or is not readable`); diff --git a/src/common/Page.ts b/src/common/Page.ts index 4000415c2f9..a6b2146ccf2 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import * as fs from 'fs'; -import { promisify } from 'util'; import { EventEmitter } from './EventEmitter.js'; import { Connection, @@ -57,8 +55,7 @@ import { UnwrapPromiseLike, } from './EvalTypes.js'; import { PDFOptions, paperFormats } from './PDFOptions.js'; - -const writeFileAsync = promisify(fs.writeFile); +import { isNode } from '../environment.js'; /** * @public @@ -1750,7 +1747,13 @@ export class Page extends EventEmitter { options.encoding === 'base64' ? result.data : Buffer.from(result.data, 'base64'); - if (options.path) await writeFileAsync(options.path, buffer); + if (!isNode && options.path) { + throw new Error( + 'Screenshots can only be written to a file path in a Node environment.' + ); + } + const fs = await import('fs'); + if (options.path) await fs.promises.writeFile(options.path, buffer); return buffer; function processClip( diff --git a/src/common/helper.ts b/src/common/helper.ts index 218c41c261d..72c51c0ee0a 100644 --- a/src/common/helper.ts +++ b/src/common/helper.ts @@ -17,15 +17,10 @@ import { TimeoutError } from './Errors.js'; import { debug } from './Debug.js'; import * as fs from 'fs'; import { CDPSession } from './Connection.js'; -import { promisify } from 'util'; import { Protocol } from 'devtools-protocol'; import { CommonEventEmitter } from './EventEmitter.js'; import { assert } from './assert.js'; -const openAsync = promisify(fs.open); -const writeAsync = promisify(fs.write); -const closeAsync = promisify(fs.close); - export const debugError = debug('puppeteer:error'); function getExceptionMessage( @@ -207,8 +202,10 @@ async function readProtocolStream( path?: string ): Promise { let eof = false; - let file; - if (path) file = await openAsync(path, 'w'); + let fileHandle: fs.promises.FileHandle; + if (path) { + fileHandle = await fs.promises.open(path, 'w'); + } const bufs = []; while (!eof) { const response = await client.send('IO.read', { handle }); @@ -218,9 +215,9 @@ async function readProtocolStream( response.base64Encoded ? 'base64' : undefined ); bufs.push(buf); - if (path) await writeAsync(file, buf); + if (path) await fs.promises.writeFile(fileHandle, buf); } - if (path) await closeAsync(file); + if (path) await fileHandle.close(); await client.send('IO.close', { handle }); let resultBuffer = null; try { diff --git a/test/tsconfig.test.json b/test/tsconfig.test.json index 6ae022f65bf..3432441200f 100644 --- a/test/tsconfig.test.json +++ b/test/tsconfig.test.json @@ -1,3 +1,6 @@ { "extends": "../tsconfig.base.json", + "compilerOptions": { + "module": "CommonJS" + } } diff --git a/tsconfig.base.json b/tsconfig.base.json index 31a7a463db0..97d1cd068a4 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -5,6 +5,7 @@ "checkJs": true, "target": "ESNext", "moduleResolution": "node", + "module": "ESNext", "declaration": true, "declarationMap": true, "resolveJsonModule": true