chore: vendor Mitt & update project structure (#6209)

* chore: vendor Mitt into src/common/third-party

As discussed in #6203 we need to vendor our common dependencies in so
that when we ship an ESM build all imports point to file paths and do
not rely on Node resolution (e.g. a browser does not understand `import
mitt from 'mitt'`).

.eslintignore

@@ -10,3 +10,4 @@ lib/
 # We ignore this file because it uses ES imports which we don't yet use
 # in the Puppeteer src, so it trips up the ESLint-TypeScript parser.
 utils/doclint/generate_types/test/test.ts
+vendor/

.eslintrc.js

@@ -87,8 +87,14 @@ module.exports = {
         // enforce the variable in a catch block is named error
         "unicorn/catch-error-name": "error",
 
+
         "no-restricted-imports": ["error", {
             patterns: ["*Events"],
+            paths: [{
+                name: "mitt",
+                message:
+                "Import Mitt from the vendored location: vendor/mitt/src/index.js",
+            }],
         }],
         "import/extensions": ["error", "ignorePackages"]
     },

CONTRIBUTING.md

@@ -5,6 +5,10 @@
   * [Code reviews](#code-reviews)
   * [Code Style](#code-style)
   * [TypeScript guidelines](#typescript-guidelines)
+  * [Project structure and TypeScript compilation](#project-structure-and-typescript-compilation)
+    - [Shipping CJS and ESM bundles](#shipping-cjs-and-esm-bundles)
+    - [tsconfig for the tests](#tsconfig-for-the-tests)
+    - [Root `tsconfig.json`](#root-tsconfigjson)
   * [API guidelines](#api-guidelines)
   * [Commit Messages](#commit-messages)
   * [Writing Documentation](#writing-documentation)
@@ -85,6 +89,42 @@ npm run tsc
 
 - Try to avoid the use of `any` when possible. Consider `unknown` as a better alternative. You are able to use `any` if needbe, but it will generate an ESLint warning.
 
+## Project structure and TypeScript compilation
+
+The code in Puppeteer is split primarily into two folders:
+
+- `src` contains all source code
+- `vendor` contains all dependencies that we've vendored into the codebase. See the [`vendor/README.md`](https://github.com/puppeteer/puppeteer/blob/main/vendor/README.md) for details.
+
+We structure these using TypeScript's project references, which lets us treat each folder like a standalone TypeScript project.
+
+### Shipping CJS and ESM bundles
+
+Currently Puppeteer ships two bundles; a CommonJS version for Node and an ESM bundle for the browser. Therefore we maintain two `tsconfig` files for each project; `tsconfig.esm.json` and `tsconfig.cjs.json`. At build time we compile twice, once outputting to CJS and another time to output to ESM.
+
+We compile into the `lib` directory which is what we publish on the npm repository and it's structured like so:
+
+```
+lib
+- cjs
+  - puppeteer <== the output of compiling `src/tsconfig.cjs.json`
+  - vendor <== the output of compiling `vendor/tsconfig.cjs.json`
+- esm
+  - puppeteer <== the output of compiling `src/tsconfig.esm.json`
+  - vendor <== the output of compiling `vendor/tsconfig.esm.json`
+```
+
+The main entry point for the Node module Puppeteer is `cjs-entry.js`. This imports `lib/cjs/puppeteer/index.js` and exposes it to Node users.
+
+### tsconfig for the tests
+
+We also maintain `test/tsconfig.test.json`. This is **only used to compile the unit test `*.spec.ts` files**. When the tests are run, we first compile Puppeteer as normal before running the unit tests **against the compiled output**. Doing this lets the test run against the compiled code we ship to users so it gives us more confidence in our compiled output being correct.
+
+### Root `tsconfig.json`
+
+The root `tsconfig.json` exists for the API Extractor; it has to find a `tsconfig.json` in the project's root directory. It is _not_ used for anything else.
+
+
 ## API guidelines
 
 When authoring new API methods, consider the following:
@@ -153,6 +193,9 @@ For all dependencies (both installation and development):
 A barrier for introducing new installation dependencies is especially high:
 - **Do not add** installation dependency unless it's critical to project success.
 
+There are additional considerations for dependencies that are environment agonistic. See the [`vendor/README.md`](https://github.com/puppeteer/puppeteer/blob/main/vendor/README.md) for details.
+
+
 ## Running & Writing Tests
 
 - Every feature should be accompanied by a test.

api-extractor.json

@@ -1,6 +1,6 @@
 {
   "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
-  "mainEntryPointFilePath": "<projectFolder>/lib/cjs/api-docs-entry.d.ts",
+  "mainEntryPointFilePath": "<projectFolder>/lib/cjs/puppeteer/api-docs-entry.d.ts",
   "bundledPackages": [ "devtools-protocol" ],
 
   "apiReport": {

cjs-entry-core.js

@@ -25,5 +25,5 @@
  * This means that we can publish to CJS and ESM whilst maintaining the expected
  * import behaviour for CJS and ESM users.
  */
-const puppeteerExport = require('./lib/cjs/index-core');
+const puppeteerExport = require('./lib/cjs/puppeteer/index-core');
 module.exports = puppeteerExport.default;

cjs-entry.js

@@ -25,5 +25,5 @@
  * This means that we can publish to CJS and ESM whilst maintaining the expected
  * import behaviour for CJS and ESM users.
  */
-const puppeteerExport = require('./lib/cjs/index');
+const puppeteerExport = require('./lib/cjs/puppeteer/index');
 module.exports = puppeteerExport.default;

install.js

@@ -29,7 +29,10 @@ const compileTypeScriptIfRequired = require('./typescript-if-required');
 async function download() {
   await compileTypeScriptIfRequired();
   // need to ensure TS is compiled before loading the installer
-  const { downloadBrowser, logPolitely } = require('./lib/cjs/install');
+  const {
+    downloadBrowser,
+    logPolitely,
+  } = require('./lib/cjs/puppeteer/install');
 
   if (process.env.PUPPETEER_SKIP_DOWNLOAD) {
     logPolitely(

package.json

@@ -24,13 +24,12 @@
     "doc": "node utils/doclint/cli.js",
     "clean-lib": "rm -rf lib",
     "tsc": "npm run clean-lib && tsc --version && npm run tsc-cjs && npm run tsc-esm",
-    "tsc-cjs": "tsc -p .",
-    "tsc-esm": "tsc --build tsconfig-esm.json",
-    "typecheck": "tsc -p . --noEmit",
+    "tsc-cjs": "tsc -b src/tsconfig.cjs.json",
+    "tsc-esm": "tsc -b src/tsconfig.esm.json",
     "apply-next-version": "node utils/apply_next_version.js",
     "test-install": "scripts/test-install.sh",
     "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 && exit `git status --porcelain | head -255 | wc -l`",
+    "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"
   },
@@ -49,7 +48,6 @@
     "extract-zip": "^2.0.0",
     "https-proxy-agent": "^4.0.0",
     "mime": "^2.0.3",
-    "mitt": "^2.0.1",
     "pkg-dir": "^4.2.0",
     "progress": "^2.0.1",
     "proxy-from-env": "^1.0.0",

src/common/EventEmitter.ts

@@ -1,4 +1,8 @@
-import mitt, { Emitter, EventType, Handler } from 'mitt';
+import mitt, {
+  Emitter,
+  EventType,
+  Handler,
+} from '../../vendor/mitt/src/index.js';
 
 /**
  * @internal

src/tsconfig.cjs.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../tsconfig.base.json",
+  "compilerOptions": {
+    "composite": true,
+    "outDir": "../lib/cjs/puppeteer",
+    "module": "CommonJS"
+  },
+  "references": [
+    { "path": "../vendor/tsconfig.cjs.json"}
+  ]
+}

src/tsconfig.esm.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../tsconfig.base.json",
+  "compilerOptions": {
+    "composite": true,
+    "outDir": "../lib/esm/puppeteer",
+    "module": "esnext"
+  },
+  "references": [
+    { "path": "../vendor/tsconfig.esm.json"}
+  ]
+}

test/EventEmitter.spec.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { EventEmitter } from '../lib/cjs/common/EventEmitter.js';
+import { EventEmitter } from '../lib/cjs/puppeteer/common/EventEmitter.js';
 import sinon from 'sinon';
 import expect from 'expect';
 

test/coverage-utils.js

@@ -39,32 +39,32 @@ const fs = require('fs');
  * part of the TSDoc migration.
  */
 const MODULES_TO_CHECK_FOR_COVERAGE = {
-  Accessibility: '../lib/cjs/common/Accessibility',
-  Browser: '../lib/cjs/common/Browser',
-  BrowserContext: '../lib/cjs/common/Browser',
-  BrowserFetcher: '../lib/cjs/node/BrowserFetcher',
-  CDPSession: '../lib/cjs/common/Connection',
-  ConsoleMessage: '../lib/cjs/common/ConsoleMessage',
-  Coverage: '../lib/cjs/common/Coverage',
-  Dialog: '../lib/cjs/common/Dialog',
-  ElementHandle: '../lib/cjs/common/JSHandle',
-  ExecutionContext: '../lib/cjs/common/ExecutionContext',
-  EventEmitter: '../lib/cjs/common/EventEmitter',
-  FileChooser: '../lib/cjs/common/FileChooser',
-  Frame: '../lib/cjs/common/FrameManager',
-  JSHandle: '../lib/cjs/common/JSHandle',
-  Keyboard: '../lib/cjs/common/Input',
-  Mouse: '../lib/cjs/common/Input',
-  Page: '../lib/cjs/common/Page',
-  Puppeteer: '../lib/cjs/common/Puppeteer',
-  HTTPRequest: '../lib/cjs/common/HTTPRequest',
-  HTTPResponse: '../lib/cjs/common/HTTPResponse',
-  SecurityDetails: '../lib/cjs/common/SecurityDetails',
-  Target: '../lib/cjs/common/Target',
-  TimeoutError: '../lib/cjs/common/Errors',
-  Touchscreen: '../lib/cjs/common/Input',
-  Tracing: '../lib/cjs/common/Tracing',
-  WebWorker: '../lib/cjs/common/WebWorker',
+  Accessibility: '../lib/cjs/puppeteer/common/Accessibility',
+  Browser: '../lib/cjs/puppeteer/common/Browser',
+  BrowserContext: '../lib/cjs/puppeteer/common/Browser',
+  BrowserFetcher: '../lib/cjs/puppeteer/node/BrowserFetcher',
+  CDPSession: '../lib/cjs/puppeteer/common/Connection',
+  ConsoleMessage: '../lib/cjs/puppeteer/common/ConsoleMessage',
+  Coverage: '../lib/cjs/puppeteer/common/Coverage',
+  Dialog: '../lib/cjs/puppeteer/common/Dialog',
+  ElementHandle: '../lib/cjs/puppeteer/common/JSHandle',
+  ExecutionContext: '../lib/cjs/puppeteer/common/ExecutionContext',
+  EventEmitter: '../lib/cjs/puppeteer/common/EventEmitter',
+  FileChooser: '../lib/cjs/puppeteer/common/FileChooser',
+  Frame: '../lib/cjs/puppeteer/common/FrameManager',
+  JSHandle: '../lib/cjs/puppeteer/common/JSHandle',
+  Keyboard: '../lib/cjs/puppeteer/common/Input',
+  Mouse: '../lib/cjs/puppeteer/common/Input',
+  Page: '../lib/cjs/puppeteer/common/Page',
+  Puppeteer: '../lib/cjs/puppeteer/common/Puppeteer',
+  HTTPRequest: '../lib/cjs/puppeteer/common/HTTPRequest',
+  HTTPResponse: '../lib/cjs/puppeteer/common/HTTPResponse',
+  SecurityDetails: '../lib/cjs/puppeteer/common/SecurityDetails',
+  Target: '../lib/cjs/puppeteer/common/Target',
+  TimeoutError: '../lib/cjs/puppeteer/common/Errors',
+  Touchscreen: '../lib/cjs/puppeteer/common/Input',
+  Tracing: '../lib/cjs/puppeteer/common/Tracing',
+  WebWorker: '../lib/cjs/puppeteer/common/WebWorker',
 };
 
 function traceAPICoverage(apiCoverage, className, modulePath) {

test/elementhandle.spec.ts

@@ -24,7 +24,7 @@ import {
 } from './mocha-utils'; // eslint-disable-line import/extensions
 
 import utils from './utils.js';
-import { ElementHandle } from '../lib/cjs/common/JSHandle.js';
+import { ElementHandle } from '../lib/cjs/puppeteer/common/JSHandle.js';
 
 describe('ElementHandle specs', function () {
   setupTestBrowserHooks();

test/keyboard.spec.ts

@@ -23,7 +23,7 @@ import {
   setupTestPageAndContextHooks,
   itFailsFirefox,
 } from './mocha-utils'; // eslint-disable-line import/extensions
-import { KeyInput } from '../lib/cjs/common/USKeyboardLayout.js';
+import { KeyInput } from '../lib/cjs/puppeteer/common/USKeyboardLayout.js';
 
 describe('Keyboard', function () {
   setupTestBrowserHooks();

test/launcher.spec.ts

@@ -27,7 +27,7 @@ import {
 import utils from './utils.js';
 import expect from 'expect';
 import rimraf from 'rimraf';
-import { Page } from '../lib/cjs/common/Page.js';
+import { Page } from '../lib/cjs/puppeteer/common/Page.js';
 
 const rmAsync = promisify(rimraf);
 const mkdtempAsync = promisify(fs.mkdtemp);

test/mocha-ts-require.js

@@ -1,8 +1,11 @@
+const path = require('path');
+
 require('ts-node').register({
   /**
    * We ignore the lib/ directory because that's already been TypeScript
    * compiled and checked. So we don't want to check it again as part of running
    * the unit tests.
    */
   ignore: ['lib/*', 'node_modules'],
+  project: path.join(__dirname, 'tsconfig.test.json'),
 });

test/mocha-utils.ts

@@ -19,10 +19,13 @@ import * as path from 'path';
 import * as fs from 'fs';
 import * as os from 'os';
 import sinon from 'sinon';
-import puppeteer from '../lib/cjs/index.js';
-import { Browser, BrowserContext } from '../lib/cjs/common/Browser.js';
-import { Page } from '../lib/cjs/common/Page.js';
-import { Puppeteer } from '../lib/cjs/common/Puppeteer.js';
+import puppeteer from '../lib/cjs/puppeteer/index.js';
+import {
+  Browser,
+  BrowserContext,
+} from '../lib/cjs/puppeteer/common/Browser.js';
+import { Page } from '../lib/cjs/puppeteer/common/Page.js';
+import { Puppeteer } from '../lib/cjs/puppeteer/common/Puppeteer.js';
 import utils from './utils.js';
 import rimraf from 'rimraf';
 

test/mouse.spec.ts

@@ -21,7 +21,7 @@ import {
   setupTestPageAndContextHooks,
   itFailsFirefox,
 } from './mocha-utils'; // eslint-disable-line import/extensions
-import { KeyInput } from '../lib/cjs/common/USKeyboardLayout.js';
+import { KeyInput } from '../lib/cjs/puppeteer/common/USKeyboardLayout.js';
 
 interface Dimensions {
   x: number;

test/page.spec.ts

@@ -26,8 +26,8 @@ import {
   itFailsFirefox,
   describeFailsFirefox,
 } from './mocha-utils'; // eslint-disable-line import/extensions
-import { Page, Metrics } from '../lib/cjs/common/Page.js';
-import { JSHandle } from '../lib/cjs/common/JSHandle.js';
+import { Page, Metrics } from '../lib/cjs/puppeteer/common/Page.js';
+import { JSHandle } from '../lib/cjs/puppeteer/common/JSHandle.js';
 
 describe('Page', function () {
   setupTestBrowserHooks();

test/target.spec.ts

@@ -23,7 +23,7 @@ import {
   setupTestPageAndContextHooks,
   itFailsFirefox,
 } from './mocha-utils'; // eslint-disable-line import/extensions
-import { Target } from '../lib/cjs/common/Target.js';
+import { Target } from '../lib/cjs/puppeteer/common/Target.js';
 
 describe('Target', function () {
   setupTestBrowserHooks();

test/tsconfig.test.json

@@ -0,0 +1,3 @@
+{
+  "extends": "../tsconfig.base.json",
+}

test/worker.spec.ts

@@ -22,8 +22,8 @@ import {
   describeFailsFirefox,
 } from './mocha-utils'; // eslint-disable-line import/extensions
 import utils from './utils.js';
-import { WebWorker } from '../lib/cjs/common/WebWorker.js';
-import { ConsoleMessage } from '../lib/cjs/common/ConsoleMessage.js';
+import { WebWorker } from '../lib/cjs/puppeteer/common/WebWorker.js';
+import { ConsoleMessage } from '../lib/cjs/puppeteer/common/ConsoleMessage.js';
 const { waitEvent } = utils;
 
 describeFailsFirefox('Workers', function () {

tsconfig.base.json

@@ -0,0 +1,12 @@
+{
+  "compilerOptions": {
+    "esModuleInterop": true,
+    "allowJs": true,
+    "checkJs": true,
+    "target": "ESNext",
+    "moduleResolution": "node",
+    "declaration": true,
+    "declarationMap": true,
+    "resolveJsonModule": true
+  }
+}

tsconfig.json

@@ -1,17 +1,11 @@
+/**
+ * This configuration only exists for the API Extractor tool. See the details in
+ * CONTRIBUTING.md that describes our TypeScript setup.
+*/
 {
+  "extends": "./tsconfig.base.json",
   "compilerOptions": {
-    "allowJs": true,
-    "checkJs": true,
-    "outDir": "./lib/cjs",
-    "target": "ESNext",
-    "moduleResolution": "node",
-    "module": "CommonJS",
-    "declaration": true,
-    "declarationMap": true,
-    "esModuleInterop": true,
-    "resolveJsonModule": true
+    "noEmit": true
   },
-  "include": [
-    "src"
-  ]
+  "include": ["src"]
 }