From 88d843d4f0b7ba389cb15cb4cec08d42a4de3ae8 Mon Sep 17 00:00:00 2001
From: Jack Franklin <jacktfranklin@chromium.org>
Date: Tue, 14 Apr 2020 10:55:29 +0100
Subject: [PATCH] feat(TypeScript): move DeviceDescriptors to TS (#5595)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit moves `src/DeviceDescriptors` to be authored in TypeScript. This file was chosen due to its simplicity so that we can focus on getting a mixed JS/TS codebase playing nicely before migrating the more complex files.

The file itself was a bit odd: although the array of devices was exported via `module.exports` that was never referenced by any consumers; each device was also exported via `module.exports[name] = device` and that is how it's consumed. The Puppeteer docs suggest using it like so:

```js
puppeteer.devices['iPhone 6']
```

So instead of exporting the array and then setting a bunch of properties on that, we instead define the array and export an object of keys where each key is a device. This is a breaking change (see the footer for details).

Rather than export an object I'd much rather export a Map, but that would be a larger breaking change and I'm keen to avoid those for the time being.

Note that we have to use special TypeScript specific syntax for the export that enables it to work in a CommonJS codebase [1] but again I'd rather this than move to ESM at this time. TypeScript still outputs CommonJS into `lib/` as you would expect.

BREAKING CHANGE: We no longer export an array of devices, so any users relying on doing:

```js
puppeter.devices.forEach(...)
```

…will now see a breakage. The fix is to use `Object.{keys/entries/values}` to iterate instead.

[1]: https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require
---
 ...iceDescriptors.js => DeviceDescriptors.ts} | 29 ++++++++++++--
 tsconfig.json                                 |  3 +-
 utils/doclint/cli.js                          | 40 +++++++++----------
 3 files changed, 47 insertions(+), 25 deletions(-)
 rename src/{DeviceDescriptors.js => DeviceDescriptors.ts} (98%)

diff --git a/src/DeviceDescriptors.js b/src/DeviceDescriptors.ts
similarity index 98%
rename from src/DeviceDescriptors.js
rename to src/DeviceDescriptors.ts
index 5d586d779a455..945e5cadf7f47 100644
--- a/src/DeviceDescriptors.js
+++ b/src/DeviceDescriptors.ts
@@ -14,7 +14,20 @@
  * limitations under the License.
  */
 
-module.exports = [
+interface Device {
+  name: string;
+  userAgent: string;
+  viewport: {
+    width: number;
+    height: number;
+    deviceScaleFactor: number;
+    isMobile: boolean;
+    hasTouch: boolean;
+    isLandscape: boolean;
+  };
+};
+
+const devices: Device[] = [
   {
     'name': 'Blackberry PlayBook',
     'userAgent': 'Mozilla/5.0 (PlayBook; U; RIM Tablet OS 2.1.0; en-US) AppleWebKit/536.2+ (KHTML like Gecko) Version/7.2.1.0 Safari/536.2+',
@@ -868,5 +881,15 @@ module.exports = [
     }
   }
 ];
-for (const device of module.exports)
-  module.exports[device.name] = device;
+
+type DevicesMap = {
+  [name: string]: Device;
+};
+
+const devicesMap: DevicesMap = {};
+
+for (const device of devices) {
+  devicesMap[device.name] = device;
+}
+
+export = devicesMap;
diff --git a/tsconfig.json b/tsconfig.json
index 815fab9b1be10..baf9d96a5a493 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -4,7 +4,8 @@
     "checkJs": true,
     "outDir": "./lib",
     "target": "ESNext",
-    "moduleResolution": "node"
+    "moduleResolution": "node",
+    "module": "CommonJS"
   },
   "include": [
     "src"
diff --git a/utils/doclint/cli.js b/utils/doclint/cli.js
index ed58a4b43a5cf..96eb000a723f5 100755
--- a/utils/doclint/cli.js
+++ b/utils/doclint/cli.js
@@ -36,31 +36,29 @@ async function run() {
   let changedFiles = false;
 
   // Documentation checks.
-  {
-    const readme = await Source.readFile(path.join(PROJECT_DIR, 'README.md'));
-    const contributing = await Source.readFile(path.join(PROJECT_DIR, 'CONTRIBUTING.md'));
-    const api = await Source.readFile(path.join(PROJECT_DIR, 'docs', 'api.md'));
-    const troubleshooting = await Source.readFile(path.join(PROJECT_DIR, 'docs', 'troubleshooting.md'));
-    const mdSources = [readme, api, troubleshooting, contributing];
+  const readme = await Source.readFile(path.join(PROJECT_DIR, 'README.md'));
+  const contributing = await Source.readFile(path.join(PROJECT_DIR, 'CONTRIBUTING.md'));
+  const api = await Source.readFile(path.join(PROJECT_DIR, 'docs', 'api.md'));
+  const troubleshooting = await Source.readFile(path.join(PROJECT_DIR, 'docs', 'troubleshooting.md'));
+  const mdSources = [readme, api, troubleshooting, contributing];
 
-    const preprocessor = require('./preprocessor');
-    messages.push(...await preprocessor.runCommands(mdSources, VERSION));
-    messages.push(...await preprocessor.ensureReleasedAPILinks([readme], VERSION));
+  const preprocessor = require('./preprocessor');
+  messages.push(...await preprocessor.runCommands(mdSources, VERSION));
+  messages.push(...await preprocessor.ensureReleasedAPILinks([readme], VERSION));
 
-    const browser = await puppeteer.launch();
-    const page = await browser.newPage();
-    const checkPublicAPI = require('./check_public_api');
-    const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib'));
-    messages.push(...await checkPublicAPI(page, mdSources, jsSources));
+  const browser = await puppeteer.launch();
+  const page = await browser.newPage();
+  const checkPublicAPI = require('./check_public_api');
+  const jsSources = await Source.readdir(path.join(PROJECT_DIR, 'lib'));
+  messages.push(...await checkPublicAPI(page, mdSources, jsSources));
 
-    await browser.close();
+  await browser.close();
 
-    for (const source of mdSources) {
-      if (!source.hasUpdatedText())
-        continue;
-      await source.save();
-      changedFiles = true;
-    }
+  for (const source of mdSources) {
+    if (!source.hasUpdatedText())
+      continue;
+    await source.save();
+    changedFiles = true;
   }
 
   // Report results.