fix: much better TypeScript definitions

[jackfranklin]

This PR aims to vastly improve our TS types and how we ship them.

Our previous attempt at shipping TypeScript was unfortunately flawed for
many reasons when compared to the @types/puppeteer package:

• It only worked if you needed the default export. If you wanted to
  import a type that Puppeteer uses, you'd have to do import type X from 'puppeteer/lib/...'. This is not something we want to encourage
  because that means our internal file structure becomes almost public
  API.
• It gave absolutely no help to CommonJS users in JS files because it
  would warn people they needed to do `const pptr =
  require('puppeteer').default, which is not correct.
• I found a bug in the evaluate types which mean't you couldn't
  override the types to provide more info, and TS would insist the types
  were all unknown.

The goal of this PR is to support:

1. In a ts file, import puppeteer from 'puppeteer'
2. In a ts file, import type {ElementHandle} from 'puppeteer'
3. In a ts file, referencing a type as puppeteer.ElementHandle
4. In a ts file, you can get good type inference when running
   foo.evaluate(x => x.clientHeight).
5. In a js file using CJS, you can do const puppeteer = require('puppeteer') and get good type help from VSCode.

To test this I created a new empty repository with two test files in,
one .ts file with this in:
https://gist.github.com/jackfranklin/22ba2f390f97c7312cd70025a2096fc8,
and a js file with this in:
https://gist.github.com/jackfranklin/06bed136fdb22419cb7a8a9a4d4ef32f.

These files included enough code to check that the types were behaving
as I expected.

The fix for our types was to make use of API Extractor, which we already
use for our docs, to "rollup" all the disparate type files that TS
generates into one large types.d.ts which contains all the various
types that we define, such as:

export declare class ElementHandle {...}

export type EvaluateFn ...

If we then update our package.json types field to point to that file
in lib/types.d.ts, this then allows a developer to write:

import type {ElementHandle} from 'puppeteer'

And get the correct type definitions. However, what the types.d.ts
file doesn't do out of the box is declare the default export, so
importing Puppeteer's default export to call a method such as launch
on it will get you an error.

That's where the script/add-default-export-to-types.ts comes in. It
appends the following to the auto-generated types.d.ts file:

declare const puppeteer: PuppeteerNode;
export = puppeteer;

This tells TypeScript what the default export is, and by using the
export = syntax, we make sure TS understands both in a TS ESM
environment and in a JS CJS environment.

Now the build step, which is run by GitHub Actions when we release,
will generate the .d.ts file and then extend it with the default
export code.

To ensure that I was generating a valid package, I created a new
repository locally with the two code samples linked in Gists above. I
then ran:

npm init -y
npm install --save-dev typescript
npx tsc --init

Which gives me a base to test from. In Puppeteer, I ran npm pack,
which packs the module into a tar that's almost identical to what would
be published, so I can be confident that the .d.ts files in there are
what would be published.

I then installed it:

npm install --save-dev ../../puppeteer/puppeteer-7.0.1-post.tgz

And then reloaded VSCode in my dummy project. By deliberately making
typos and hovering over the code, I could confirm that all the goals
listed above were met, and this seems like a vast improvement on our
types.

[jackfranklin]

@mathiasbynens LMK what you think. This should help improve the TS situation for a lot of users (#6827 as an example).

[mathiasbynens]

Nice!

[danielrentz]

Please note (and mention in the docs) that TypeScript does not allow to import an "object export" like

export = puppeteer;

as

import puppeteer from "puppeteer";

out of the box.

Instead, it is required to write

import * as puppeteer from "puppeteer";

or

import puppeteer = require("puppeteer");

See https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require.

However, there is a compiler option allowSyntheticDefaultImports which will allow to use the "default import" form for export objects, but libraries should not rely on that all users will have it on.

[danielrentz]

BTW, also missing in the patch is an update of the main README. It still recommends using the @types package.

[jackfranklin]

@danielrentz thanks for your comments.

I think it's OK to ask people to either turn on the ESM interop flag or to do import * as puppeteer. The alternative is to switch to export default puppeteer in our type defs but then CJS JS users get no type info from VSCode, so I don't think that's a great idea.

It looks like the @types/puppeteer package also asks you to either turn on the interop flag or do import * as based on my testing. This is on the DevTools Frontend repo on an old Puppeteer before we shipped our types:

So requiring the interop flag or import * as is not a change and I think is the best path forwards for now. In the future once Node 10 is EOL'd and we can move to supporting just ES Modules, we can improve the situation I expect.

[SimenB]

this types.d.ts file isn't published.

https://www.runpkg.com/?puppeteer@7.0.3

[SimenB]

possibly "lib/**/*.d.ts" below doesn't include it since its not nested?

[mathiasbynens]

Jack fixed this in a subsequent patch release. Does v7.0.3 work for you, @SimenB?

[SimenB]

Nope, I linked to 7.0.3

[SimenB]

I see #6844 now, doesn't seem like it's enough

[SimenB]

Last one, maybe #6848 fixed it? No release has been made since it landed

[mathiasbynens]

Apologies, I had missed your link earlier. Thanks for bearing with us here @SimenB. @jackfranklin is cutting a release soon.

[SimenB]

That works, thanks!

New issue, though

../../node_modules/puppeteer/lib/types.d.ts:24385:1 - error TS2309: An export assignment cannot be used in a module with other exported elements.

24385 export = puppeteer;
      ~~~~~~~~~~~~~~~~~~~

Using TypeScript 4.1.3 fwiw

(note that 7.0.1 works for us, although we don't need any of the exported types here)

---

What we do in Jest where we're stuck with export = is to use a namespace to also export types. E.g. https://github.com/facebook/jest/blob/v26.6.3/packages/jest-resolve/src/index.ts#L37-L43 & https://github.com/facebook/jest/blob/v26.6.3/packages/jest-resolve/src/index.ts#L505

That's in a source ts file, but I assume it's almost the same in d.ts files - https://www.runpkg.com/?jest-resolve@26.6.2/build/index.d.ts

[danielrentz]

I suggest to export the puppeteer object as default export and as object export in cjs-entry.js, e.g.

const puppeteerExport = require('./lib/cjs/puppeteer/node');
module.exports = puppeteerExport.default;
module.exports.default = puppeteerExport.default;

and to define the puppeteer object as default export in the types.d.ts only (export default puppeteer;). TSC will still accept import puppeteer from "puppeteer"; then (default import), and in CJS it is still possible to use const puppeteer = require("puppeteer"); (object import).

[jackfranklin]

@SimenB thank you for your help 👍 The namespace idea looks great, I'm looking at creating that PR now.