chore: use ts-doc and factor out importFSModule (#8449)

.eslintrc.js

@@ -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',

package.json

@@ -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",

scripts/ensure-correct-devtools-protocol-package.ts

@@ -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.

src/.eslintrc.js

@@ -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: {

src/common/Connection.ts

@@ -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,
@@ -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,

src/common/DOMWorld.ts

@@ -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();
@@ -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();
@@ -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)

src/common/Events.ts

@@ -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 = {

src/common/JSHandle.ts

@@ -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) => {

src/common/LifecycleWatcher.ts

@@ -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[]

src/common/Page.ts

@@ -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();
+      const fs = (await import('fs')).promises;
-      await fs.promises.writeFile(options.path, buffer);
+      await fs.writeFile(options.path, buffer);
     }
     return buffer;
 
@@ -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) {

src/common/assert.ts

@@ -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 = (

src/common/helper.ts

@@ -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);
     }
   }
 
@@ -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,
@@ -413,7 +389,6 @@ export const helper = {
   waitForEvent,
   isString,
   isNumber,
-  importFSModule,
   addEventListener,
   removeEventListeners,
   valueFromRemoteObject,

src/node/BrowserRunner.ts

@@ -277,9 +277,6 @@ function waitForWSEndpoint(
     ];
     const timeoutId = timeout ? setTimeout(onTimeout, timeout) : 0;
 
-    /**
-     * @param {!Error=} error
-     */
     function onClose(error?: Error): void {
       cleanup();
       reject(

src/node/Launcher.ts

@@ -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 prefs - List of preferences to add.
-   * @param profilePath Firefox profile to write the preferences to.
+   * @param profilePath - Firefox profile to write the preferences to.
    */
   async writePreferences(
     prefs: { [x: string]: unknown },

test/NetworkManager.spec.ts

@@ -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();

test/oopif.spec.ts

@@ -419,9 +419,6 @@ describeChromeOnly('OOPIF', function () {
   });
 });
 
-/**
- * @param {!BrowserContext} context
- */
 function oopifs(context) {
   return context
     .targets()

test/requestinterception.spec.ts

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