Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: much better TypeScript definitions #6837

Merged
merged 1 commit into from
Feb 9, 2021
Merged

fix: much better TypeScript definitions #6837

merged 1 commit into from
Feb 9, 2021

Commits on Feb 9, 2021

  1. fix: much better TypeScript definitions

    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'`
    1. In a `ts` file, `import type {ElementHandle} from 'puppeteer'`
    1. In a `ts` file, referencing a type as `puppeteer.ElementHandle`
    1. In a `ts` file, you can get good type inference when running
       `foo.evaluate(x => x.clientHeight)`.
    1. 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:
    
    ```ts
    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:
    
    ```ts
    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 committed Feb 9, 2021
    Configuration menu
    Copy the full SHA
    2223130 View commit details
    Browse the repository at this point in the history