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

[6.0.0+] New Typings break existing Typescript Code #6979

Closed
1nVitr0 opened this issue Mar 12, 2021 · 8 comments
Closed

[6.0.0+] New Typings break existing Typescript Code #6979

1nVitr0 opened this issue Mar 12, 2021 · 8 comments
Assignees

Comments

@1nVitr0
Copy link

1nVitr0 commented Mar 12, 2021

Steps to reproduce

Tell us about your environment:

  • Puppeteer version: >6.0.0
  • Platform / OS version: any
  • URLs (if applicable):
  • Node.js version: any

What steps will reproduce the problem?

Please include code that reproduces the issue.

  1. Use e.g. the LaunchOptions type
  2. Upgrade to >6.0.0
  3. Enjoy type Errors

What is the expected result?

Instead of just redefining, merging, or renaming types, original types should be kept and possibly extended or split. At the very least I would expect some clear documentation what types changed.

What happens instead?

Types have been renamed, moved, merged or broken without any clear indication in the changelogs. As an example:

LaunchOptions has been reduced to a subset of it's original interface and is now extended with

LaunchOptions & BrowserLaunchArgumentOptions  & BrowserConnectOptions & {
    product?: Product;
    extraPrefsFirefox?: Record<string, unknown>;
}

There is also a PuppeteerNodeLaunchOptions which is used for the ProductLauncher interface and is pretty close to the original LaunchOptions (LaunchOptions & BrowserLaunchArgumentOptions & BrowserConnectOptions).

What I don't understand is why LaunchOptions hasn't been kept and instead been split into the before mentioned parts. This is quite a breaking change nor does it make the typings any cleaner or easier to use.

This is justn an example, see below comment for complet(ish)l list


I'm sure there might be a duplicate of this issue, but with a quick search I could only find some issues on broken typings. These renamed types are critical for projects like puppeteer-extra berstend/puppeteer-extra#428. If there is a duplicate, I'm sorry, but I don't have the time to screen 1500 open issues atm.

Anyway, I'd love to keep using the new versions of puppeteer, but right now the typings are a pretty big obstacle.

@1nVitr0
Copy link
Author

1nVitr0 commented Mar 13, 2021

For the sake of completeness as of Release 8.0.0:

The following types have been renamed, merged or in other ways changed and are no longer available:

As in all of these types throw the error Module '"puppeteer"' has no exported member 'XXX'.ts(2305)

import {
    AXNode,
    AuthOptions,
    Base64ScreenShotOptions,
    BinaryScreenShotOptions,
    Box,
    BrowserContextEventObj,
    BrowserEventObj,
    BrowserOptions,
    ChromeArgOptions,
    ConnectionTransport,
    Cookie,
    DeleteCookie,
    DialogType,
    DirectNavigationOptions,
    EmulateOptions,
    Evalable,
    FetcherOptions,
    FrameBase,
    GeoOptions,
    Headers,
    HttpMethod,
    JSEvalable,
    LayoutDimension,
    LoadEvent,
    MediaFeature,
    MediaType,
    MouseButtons,
    MousePressOptions,
    NavigationOptions,
    Overrides,
    PDFFormat,
    PageCloseOptions,
    PageEventObj,
    PageFnOptions,
    RemoteInfo,
    Request,
    RespondOptions,
    Response,
    RevisionInfo,
    SameSiteSetting,
    ScriptTagOptions,
    SetCookie,
    StartCoverageOptions,
    StyleTagOptions,
    TargetAwaiter,
    TargetType,
    Timeoutable,
    TracingStartOptions,
    WaitForSelectorOptionsHidden,
    Worker,
} from 'puppeteer';

The following Types have changes that may break existing TypeScript implementations:

I use "may" because many of them are useful changes and some may just reflect added features, some deeper analysis may be required.

Accessibility
Browser
BrowserContext
CDPSession
ConsoleMessage
ConsoleMessageType
ElementHandle
ExecutionContext
Frame
JSHandle
JSONArray
JSONObject
LaunchOptions
Metrics
PDFOptions
Page
Permission
ResourceType
Serializable
SerializableOrJSHandle
Target
UnwrapElementHandle

With LaunchOptions missing the previous properties:

{
    ignoreHTTPSErrors?: boolean;
    defaultViewport?: {
        width?: number;
        height?: number;
        deviceScaleFactor?: number;
        isMobile?: boolean;
        hasTouch?: boolean;
        isLandscape?: boolean;
    };
    slowMo?: number;
    args?: string[];
    headless?: boolean;
    userDataDir?: string;
    devtools?: boolean;
}

and some changes to ElementHandle that I'm not able to pin down that may break many of the above types.

Method of evaluation

I evaluated these changes by importing the latest versions both @types/puppeteer and puppteer. I then imported all exported typings from @types/puppeteer and checked for exports in puppeteer with the same name.

I the used a very basic Utility Type to check alignments between the types existing in both packages:

type Extends<A, B extends A> = 1;

I checked both the unchanged properties as well as a Required version of the interfaces to catch missing optional properties and let TypeScript figure out the rest:

type TestAccessibility = Extends<TypesAccessibility, Accessibility>;
type TestRequiredAccessibility = Extends<Required<TypesAccessibility>, Required<Accessibility>>;

This gave me somewhat useful Errors, e.g. the above snippet told me, that SnapshopOptions has been renamed to SnapshotOptions (which is probably a good thing, but not documented anywhere):

 Type 'import("node_modules/puppeteer/lib/types").Accessibility' does not satisfy the constraint 'import("ccccccgentiecbfvigkdfecdnerutllehlbgcrbedfju
node_modules/@types/puppeteer/index").Accessibility'.
  Types of property 'snapshot' are incompatible.
    Type '(options?: SnapshotOptions) => Promise<SerializedAXNode>' is not assignable to type '(options?: SnapshopOptions) => Promise<AXNode>'.
      Types of parameters 'options' and 'options' are incompatible.
        Type 'SnapshopOptions' is not assignable to type 'SnapshotOptions'.
          Types of property 'root' are incompatible.
            Type 'ElementHandle<Element>' is missing the following properties from type 'ElementHandle<Element>': _page, _frameManager, _scrollIntoViewIfNeeded, _clickablePoint, and 7 more.ts(2344)

Some other types, like ElementHandle may require a deeper analysis figure out what TypeScript is complayining about.

Sourcecode

See the gist for the "implementaion" of the checks I did https://gist.github.com/1nVitr0/366a5829ac432597631ecb6de9f5df89.

@radoslavkarlik
Copy link

Importing puppeteerErrors or TimeoutError also do not work.

@simonhaenisch
Copy link

simonhaenisch commented Apr 10, 2021

I'm a package author and interested in this issue, o rather in the general stability of the public (non-internal) type definitions that are exposed now. In my package.json I've been having "puppeteer": ">=2.0.0" because the APIs that my package uses have been stable since that version. This allows people to use my package with an existing puppeteer dependency in their project.

However I started exposing scripts: FrameAddScriptTagOptions[] in my tool's config recently, and will bump the dependency to >=6.0.0. Is there some kind of guarantee that changes to this or other interfaces will be considered a breaking change, and are mentioned in the changelog?

Small update: I just tried 6.0.0 and it doesn't actually expose the types, because the declaration file was only added in 7.0.1 (#6811). So I'll bump my dependency to >=7.0.1 then.

@ptommasi
Copy link
Contributor

I'm having the same issue, at the moment the workaround I found is to use:

const puppeteer = require('puppeteer-extra');
puppeteer.use(require("puppeteer-extra-plugin-stealth")());
puppeteer.use(require("puppeteer-extra-plugin-block-resources")({ "blockedTypes": new Set(["stylesheet", "font", "texttrack", "other", "image", "xhr", "media" ]) }));

Instead of importing it (basically I'm losing the typing in order to make TypeScript happy).

If I import puppeteer (or { PuppeteerExtra }) from "puppeteer-extra", this is the message I get:

> tsc && node --trace-warnings dist/app.js 

node_modules/puppeteer-extra/dist/puppeteer.d.ts:4:10 - error TS2305: Module '"puppeteer"' has no exported member 'ChromeArgOptions'.

4 export { ChromeArgOptions } from 'puppeteer';
           ~~~~~~~~~~~~~~~~

node_modules/puppeteer-extra/dist/puppeteer.d.ts:6:10 - error TS2305: Module '"puppeteer"' has no exported member 'FetcherOptions'.

6 export { FetcherOptions } from 'puppeteer';
           ~~~~~~~~~~~~~~

I'd like to use two instances of puppeteer extra (one with filters, one without), any idea?

@jackfranklin
Copy link
Collaborator

Hi all,

Sorry for the slow action here and silence. All this feedback is really useful; we were aware that in shipping our own types we would definitely find places where we miss things that the @types/puppeteer package exposed and it's our goal to fix them. Some renames happened because old Puppeteer names clashed with TypeScript built ins (Request is a good example) and others happened because we felt it could make them clearer. We're doing this work alongside a big drive to improve the documentation (more on that to come in the future hopefully) so some renames have been to try to make it easier to document.

Going forwards from our current version (v8), I would expect us to treat TS breaking changes just as we would treat API changes as breaking, so I hope that we can be more stable moving forwards, and I totally understand the way we rolled out the TS changes was, in hindsight, not ideal for helping people upgrade and adopt Puppeteer and also not ideal for those who maintain libraries built to work with Puppeteer. I'm sorry that this wasn't the smooth rollout we would have ideally had.

This week I'll go through the types documented in this issue and look into them to clarify if we can land PRs for the next release that fix some of them, or if I can help with renames. In the long term us shipping our own types should mean increased TS coverage going forward but it will take some time and effort to get there.

@dbryar
Copy link

dbryar commented May 4, 2021

const puppeteer = require('puppeteer-extra');
puppeteer.use(require("puppeteer-extra-plugin-stealth")());

The typings are failing for me too however I've utilised this workaround (from @ptommasi) to keep moving for now, and it is building without issue while we wait on the update as mentioned above.

@frontendphil
Copy link

I'd like to add another use case. @types/puppeteer exports things like Page, Element as interfaces which was extremely useful for us. For some testing purposes, we've wrapped the puppeteer API around a JSDOM. That speeds up the tests and makes mocking pages easier since we can specify the HTML directly in the test cases.

Our mock classes could implement the respective interface and leave out all the methods we're currently not using. The actual code that would use the real puppeteer version would still type check and everything was fine. We've essentially just changed some implementation details.

I know this is a VERY specific use case but it would be nice if we could keep it working somehow. If you think this might be a good addition I'm also willing to put in some work to extract the interfaces and export them alongside as IPage or something. WDYT?

@stale
Copy link

stale bot commented Jun 24, 2022

We're marking this issue as unconfirmed because it has not had recent activity and we weren't able to confirm it yet. It will be closed if no further activity occurs within the next 30 days.

@stale stale bot added the unconfirmed label Jun 24, 2022
@jrandolf jrandolf closed this as not planned Won't fix, can't repro, duplicate, stale Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants