fix(types): support objects with typed keys and values (microsoft#1752)

There are a few places in the API where we use objects as maps. This patch adds them to docs and the types.

For `env`, we accept booleans and numbers as well because they are often used for their string values.

Author: JoelEinbinder

docs/api.md

@@ -211,7 +211,7 @@ Indicates that the browser is connected.
     - `accuracy` <[number]> Non-negative accuracy value. Defaults to `0`.
   - `locale` <[string]> Specify user locale, for example `en-GB`, `de-DE`, etc. Locale will affect `navigator.language` value, `Accept-Language` request header value as well as number and date formatting rules.
   - `permissions` <[Array]<[string]>> A list of permissions to grant to all pages in this context. See [browserContext.grantPermissions](#browsercontextgrantpermissionspermissions-options) for more details.
-  - `extraHTTPHeaders` <[Object]> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
+  - `extraHTTPHeaders` <[Object]<[string], [string]>> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
   - `offline` <[boolean]> Whether to emulate network being offline. Defaults to `false`.
   - `httpCredentials` <[Object]> Credentials for [HTTP authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication).
     - `username` <[string]>
@@ -253,7 +253,7 @@ Creates a new browser context. It won't share cookies/cache with other browser c
     - `accuracy` <[number]> Non-negative accuracy value. Defaults to `0`.
   - `locale` <[string]> Specify user locale, for example `en-GB`, `de-DE`, etc. Locale will affect `navigator.language` value, `Accept-Language` request header value as well as number and date formatting rules.
   - `permissions` <[Array]<[string]>> A list of permissions to grant to all pages in this context. See [browserContext.grantPermissions](#browsercontextgrantpermissionspermissions-options) for more details.
-  - `extraHTTPHeaders` <[Object]> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
+  - `extraHTTPHeaders` <[Object]<[string], [string]>> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
   - `offline` <[boolean]> Whether to emulate network being offline. Defaults to `false`.
   - `httpCredentials` <[Object]> Credentials for [HTTP authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication).
     - `username` <[string]>
@@ -544,7 +544,7 @@ This setting will change the default maximum time for all the methods accepting
 > **NOTE** [`page.setDefaultNavigationTimeout`](#pagesetdefaultnavigationtimeouttimeout), [`page.setDefaultTimeout`](#pagesetdefaulttimeouttimeout) and [`browserContext.setDefaultNavigationTimeout`](#browsercontextsetdefaultnavigationtimeouttimeout) take priority over [`browserContext.setDefaultTimeout`](#browsercontextsetdefaulttimeouttimeout).
 
 #### browserContext.setExtraHTTPHeaders(headers)
-- `headers` <[Object]> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
+- `headers` <[Object]<[string], [string]>> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
 - returns: <[Promise]>
 
 The extra HTTP headers will be sent with every request initiated by any page in the context. These headers are merged with page-specific extra HTTP headers set with [page.setExtraHTTPHeaders()](#pagesetextrahttpheadersheaders). If page overrides a particular header, page-specific header value will be used instead of the browser context header value.
@@ -1552,7 +1552,7 @@ This setting will change the default maximum time for all the methods accepting
 > **NOTE** [`page.setDefaultNavigationTimeout`](#pagesetdefaultnavigationtimeouttimeout) takes priority over [`page.setDefaultTimeout`](#pagesetdefaulttimeouttimeout).
 
 #### page.setExtraHTTPHeaders(headers)
-- `headers` <[Object]> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
+- `headers` <[Object]<[string], [string]>> An object containing additional HTTP headers to be sent with every request. All header values must be strings.
 - returns: <[Promise]>
 
 The extra HTTP headers will be sent with every request the page initiates.
@@ -3308,7 +3308,7 @@ page.on('requestfailed', request => {
 - returns: <[Frame]> A [Frame] that initiated this request.
 
 #### request.headers()
-- returns: <[Object]> An object with HTTP headers associated with the request. All header names are lower-case.
+- returns: <[Object]<[string], [string]>> An object with HTTP headers associated with the request. All header names are lower-case.
 
 #### request.isNavigationRequest()
 - returns: <[boolean]>
@@ -3516,7 +3516,7 @@ Aborts the route's request.
 - `overrides` <[Object]> Optional request overrides, which can be one of the following:
   - `method` <[string]> If set changes the request method (e.g. GET or POST)
   - `postData` <[string]> If set changes the post data of request
-  - `headers` <[Object]> If set changes the request HTTP headers. Header values will be converted to a string.
+  - `headers` <[Object]<[string], [string]>> If set changes the request HTTP headers. Header values will be converted to a string.
 - returns: <[Promise]>
 
 Continues route's request with optional overrides.
@@ -3535,7 +3535,7 @@ await page.route('**/*', (route, request) => {
 #### route.fulfill(response)
 - `response` <[Object]> Response that will fulfill this route's request.
   - `status` <[number]> Response status code, defaults to `200`.
-  - `headers` <[Object]> Optional response headers. Header values will be converted to a string.
+  - `headers` <[Object]<[string], [string]>> Optional response headers. Header values will be converted to a string.
   - `contentType` <[string]> If set, equals to setting `Content-Type` response header.
   - `body` <[string]|[Buffer]> Optional response body.
   - `path` <[string]> Optional file path to respond with. The content type will be inferred from file extension. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
@@ -3777,7 +3777,7 @@ This methods attaches Playwright to an existing browser instance.
   - `handleSIGHUP` <[boolean]> Close the browser process on SIGHUP. Defaults to `true`.
   - `logger` <[Logger]> Logger sink for Playwright logging.
   - `timeout` <[number]> Maximum time in milliseconds to wait for the browser instance to start. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-  - `env` <[Object]> Specify environment variables that will be visible to the browser. Defaults to `process.env`.
+  - `env` <[Object]<[string], [string]|[number]|[boolean]>> Specify environment variables that will be visible to the browser. Defaults to `process.env`.
   - `devtools` <[boolean]> **Chromium-only** Whether to auto-open a Developer Tools panel for each tab. If this option is `true`, the `headless` option will be set `false`.
   - `slowMo` <[number]> Slows down Playwright operations by the specified amount of milliseconds. Useful so that you can see what is going on.
 - returns: <[Promise]<[Browser]>> Promise which resolves to browser instance.
@@ -3810,7 +3810,7 @@ const browser = await chromium.launch({  // Or 'firefox' or 'webkit'.
   - `handleSIGHUP` <[boolean]> Close the browser process on SIGHUP. Defaults to `true`.
   - `logger` <[Logger]> Logger sink for Playwright logging.
   - `timeout` <[number]> Maximum time in milliseconds to wait for the browser instance to start. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-  - `env` <[Object]> Specify environment variables that will be visible to the browser. Defaults to `process.env`.
+  - `env` <[Object]<[string], [string]|[number]|[boolean]>> Specify environment variables that will be visible to the browser. Defaults to `process.env`.
   - `devtools` <[boolean]> **Chromium-only** Whether to auto-open a Developer Tools panel for each tab. If this option is `true`, the `headless` option will be set `false`.
   - `slowMo` <[number]> Slows down Playwright operations by the specified amount of milliseconds. Useful so that you can see what is going on. Defaults to 0.
 - returns: <[Promise]<[BrowserContext]>> Promise which resolves to the browser app instance.
@@ -3829,7 +3829,7 @@ Launches browser instance that uses persistent storage located at `userDataDir`.
   - `handleSIGHUP` <[boolean]> Close the browser process on SIGHUP. Defaults to `true`.
   - `logger` <[Logger]> Logger sink for Playwright logging.
   - `timeout` <[number]> Maximum time in milliseconds to wait for the browser instance to start. Defaults to `30000` (30 seconds). Pass `0` to disable timeout.
-  - `env` <[Object]> Specify environment variables that will be visible to the browser. Defaults to `process.env`.
+  - `env` <[Object]<[string], [string]|[number]|[boolean]>> Specify environment variables that will be visible to the browser. Defaults to `process.env`.
   - `devtools` <[boolean]> **Chromium-only** Whether to auto-open a Developer Tools panel for each tab. If this option is `true`, the `headless` option will be set `false`.
 - returns: <[Promise]<[BrowserServer]>> Promise which resolves to the browser app instance.
 

src/server/browserType.ts

@@ -32,7 +32,7 @@ type LaunchOptionsBase = BrowserArgOptions & {
   handleSIGHUP?: boolean,
   timeout?: number,
   logger?: Logger,
-  env?: {[key: string]: string} | undefined
+  env?: {[key: string]: string|number|boolean}
 };
 
 export type ConnectOptions = {

src/server/processLauncher.ts

@@ -48,7 +48,7 @@ const browserStdErrLog: Log = {
 export type LaunchProcessOptions = {
   executablePath: string,
   args: string[],
-  env?: {[key: string]: string | undefined},
+  env?: {[key: string]: string | number | boolean | undefined},
 
   handleSIGINT?: boolean,
   handleSIGTERM?: boolean,
@@ -80,7 +80,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
         // process group, making it possible to kill child process tree with `.kill(-pid)` command.
         // @see https://nodejs.org/api/child_process.html#child_process_options_detached
         detached: process.platform !== 'win32',
-        env: options.env,
+        env: (options.env as {[key: string]: string}),
         stdio
       }
   );

utils/doclint/check_public_api/JSBuilder.js

@@ -184,7 +184,7 @@ function checkSources(sources) {
    */
   function serializeType(type, circular = []) {
     let typeName = checker.typeToString(type).replace(/SmartHandle/g, 'Handle');
-    if (typeName === 'any' || typeName === '{ [x: string]: string; }')
+    if (typeName === 'any') 
       typeName = 'Object';
     const nextCircular = [typeName].concat(circular);
 
@@ -196,8 +196,10 @@ function checkSources(sources) {
       }
       return new Documentation.Type(typeName, []);
     }
-
-    if (isRegularObject(type)) {
+    const stringIndexType = type.getStringIndexType();
+    if (stringIndexType) {
+      return new Documentation.Type(`Object<string, ${serializeType(stringIndexType, circular).name}>`);
+    } else if (isRegularObject(type)) {
       let properties = undefined;
       if (!circular.includes(typeName))
         properties = type.getProperties().map(property => serializeSymbol(property, nextCircular));

utils/generate_types/index.js

@@ -307,6 +307,14 @@ function parseType(type) {
 function stringifyType(parsedType) {
   if (!parsedType)
     return 'void';
+  if (parsedType.name === 'Object' && parsedType.template) {
+    const keyType = stringifyType({
+      ...parsedType.template,
+      next: null
+    });
+    const valueType = stringifyType(parsedType.template.next);
+    return `{ [key: ${keyType}]: ${valueType}; }`;
+  }
   let out = parsedType.name;
   if (parsedType.args) {
     let args = parsedType.args;

utils/generate_types/test/test.ts

@@ -135,8 +135,14 @@ playwright.chromium.launch().then(async browser => {
   await page.route(str => {
     const assertion: AssertType<string, typeof str> = true;
     return true;
-  }, interceptedRequest => {
-    interceptedRequest.continue();
+  }, (route, request) => {
+    const {referer} = request.headers();
+    const isString: AssertType<string, typeof referer> = true;
+    route.continue({
+      headers: {
+        'Access-Control-Allow-Origin': '*'
+      }
+    });
     return 'something random for no reason';
   });
 
@@ -207,6 +213,8 @@ playwright.chromium.launch().then(async browser => {
   const launchOptions: playwright.LaunchOptions = {
     devtools: true,
     env: {
+      TIMEOUT: 52,
+      SOMETHING: '/some/path',
       JEST_TEST: true
     }
   };