* chore: extract `BrowserRunner` into its own module
`src/Launcher.ts` is large and hard to work in. It has multiple objects
defined in it:
* ChromeLauncher
* FirefoxLauncher
* BrowserRunner
* Launcher
This change moves BrowserRunner into its own module. More refactorings
like this will follow but this is the first step.
* Warn when given unsupported product name.
Fixes#5844.
This change means when a user launches Puppeteer with a product name
that is not supported (which at the time of this commit means it's not
`firefox` or `chrome) we will warn them about it.
Decided on just a warning vs an error because the current behaviour is
that we fallback to launching Chrome and I don't think this warrants a
breaking change.
Now the first pass of migrating to TypeScript is complete I'm going
through the src files one by one to tidy up the public/private
interfaces.
Puppeteer used an underscore convention to denote privacy but violates
this in some places; we should be strict with TypeScript's `public` and
`private` keywords instead.
This means we'll get nice TS errors if you try to refer to a private
method/variable, and means when we swap to generating our TS docs the
tooling knows what method(s) are public and therefore need to be
documented vs private internals that don't.
* chore: Remove src/TaskQueue
The only place it's used is in `src/Page.ts` to have a chain of
screenshot promises. Rather than initialize a task queue in `Browser`
and pass it through a chain of constructors we instead move the class
into `src/Page` and define it inline.
In the future we might want to create a helpers folder to contain small
utilities like that (`src/Page.ts` is already far too large) but I'm
leaving that for a future PR.
`TaskQueue` isn't documented in `api.md` so I don't think this is a
breaking change.
I updated the type of `screenshot()` to return `Promise<string | Buffer
| void>` because if a promise rejects it's silently swallowed. I'd like
to change this behaviour but one step at a time. This type only had to
change as now we type the screenshot task queue correctly rather than
using `any` which then exposed the incorrect `screenshot()` types.
closes#5719
Original stream.pipeline issue has been fixed in Node 14.1.0 and above.
Meanwhile, the workaround itself caused timout failures in many CI Node 14 runs, as reported in #5719.
Removed it, as it's no longer needed, and actually blocks consumers from adding CI testing on Node 14.
* chore: remove src/externs.d.ts
It defined global types that we don't want to use, and instead we move
to using interfaces that we import and reference just like with any
other interface.
This means other than Protocol (which I think is fine to leave as is),
there are no other magic global types and you have to import any types
or interfaces that you want.
* chore: migrate src/Page.js to TypeScript
The final one! This is a huge file and needs to be split up and tidied,
but for now I've left all the definitions in place and converted types
accordingly.
There's some additional tidying we can do now every `src` file is TS,
but I'll leave that for another PR to avoid this one getting any bigger.
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
This change enforces how we type arrays, e.g. choosing between:
* `string[]`
* `Array<string>`
I've gone for the `array-simple` option [1] which enforces that:
* primitive types and type references use `X[]`
* complex types use `Array<X>`
For example, we'd type an array of strings as `string[]`, but an array
of a union type as `Array<SomeUnionType>`.
[1]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/array-type.md
Node's promisify function and the TS types for it give much better type understanding when wrapping a function in `promisify`. This change means we don't maintain our own implementation and our own (sub-par) types and rather lean on the tested and thorough @types/node version instead.
* chore: Useful error for Node v14 breakage
There is currently a bug with extract-zip and Node v14.0.0 that
causes extractZip to silently fail:
https://github.com/puppeteer/puppeteer/issues/5719
Rather than silenty fail if the user is on Node 14 we instead
detect that and throw an error directing the user to that bug. The
rejection message below is surfaced to the user in the command line.
The issue seems to be in streams never resolving so we wrap the call in
a timeout and give it 100ms to resolve before deciding on an error. If
the user is on Node < 14 we maintain the behaviour we had before this
patch.
Here's how this change impacts the output on Node 14 and Node 10:
Node 10:
```
npm run tsc && rm -r .local-* && node install
> puppeteer@3.0.1-post tsc /Users/jacktfranklin/src/puppeteer
> tsc --version && tsc -p . && cp src/protocol.d.ts lib/ && cp src/externs.d.ts lib/
Version 3.8.3
Downloading Chromium r737027 - 118.4 Mb [====================] 100% 0.0s
Chromium (737027) downloaded to /Users/jacktfranklin/src/puppeteer/.local-chromium/mac-737027
```
---
Node 14 without this patch:
```
npm run tsc && rm -r .local-* && node install
> puppeteer@3.0.1-post tsc /Users/jacktfranklin/src/puppeteer
> tsc --version && tsc -p . && cp src/protocol.d.ts lib/ && cp src/externs.d.ts lib/
Version 3.8.3
Downloading Chromium r737027 - 118.4 Mb [====================] 100% 0.0s
```
Note that whilst it doesn't error, it doesn't complete the install. We
don't get the success message that we saw above in the Node 10 install.
---
Node 14 with this patch:
```npm run tsc && rm -r .local-* && node install
> puppeteer@3.0.1-post tsc /Users/jacktfranklin/src/puppeteer
> tsc --version && tsc -p . && cp src/protocol.d.ts lib/ && cp src/externs.d.ts lib/
Version 3.8.3
Downloading Chromium r737027 - 118.4 Mb [====================] 100% 0.0s
ERROR: Failed to set up Chromium r737027! Set "PUPPETEER_SKIP_DOWNLOAD" env variable to skip download.
Puppeteer currently does not work on Node v14 due to an upstream bug. Please see https://github.com/puppeteer/puppeteer/issues/5719 for details.
```
The explicit message should save users a good amount of debugging time.
* chore: migrate src/Input to typescript
This moves `Keyboard`, `Mouse` and `Touchscreen` to TypeScript. We gain
some nice TS benefits here; by creating a type for all the keycodes we
support we can type the input args as that rather than `string` which
will hopefully save some users some debugging once we ship our TS types
in a future version.
* Remove from externs file
* Update utils/doclint/check_public_api/index.js
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
* chore: migrate src/ExecutionContext to TypeScript
I spent a while trying to decide on the best course of action for
typing the `evaluate` function.
Ideally I wanted to use generics so that as a user you could type
something like:
```
handle.evaluate<HTMLElement, number, boolean>((node, x) => true, 5)
```
And have TypeScript know the arguments of `node` and `x` based on those
generics. But I hit two problems with that:
* you have to have n overloads of `evaluate` to cope for as many number
of arguments as you can be bothered too (e.g. we'd need an overload for
1 arg, 2 args, 3 args, etc)
* I decided it's actually confusing because you don't know as a user
what those generics actually map to.
So in the end I went with one generic which is the return type of the
function:
```
handle.evaluate<boolean>((node, x) => true, 5)
```
And `node` and `x` get typed as `any` which means you can tell TS
yourself:
```
handle.evaluate<boolean>((node: HTMLElement, x: number) => true, 5)
```
I'd like to find a way to force that the arguments after the function do
match the arguments you've given (in the above example, TS would moan if
I swapped that `5` for `"foo"`), but I tried a few things and to be
honest the complexity of the types wasn't worth it, I don't think.
I'm very open to tweaking these but I'd rather ship this and tweak going
forwards rather than spend hours now tweaking. Once we ship these
typedefs and get feedback from the community I'm sure we can improve
them.
* chore: migrate src/JSHandle to TS
There's a few TODOs in here that all depend on typing the
`ExecutionContext.evaluateHandle` properly so that you can properly
declare what types you're expecting back. Once I've done that file (it's
next on my list) I will loop back and improve the types here, fixing
these TODOs.
* Fix doclint for {}
The codebase was incredibly inconsistent with the use of spacing around
curly braces, e.g.:
```
// this?
const a = {b: 1}
// or?
const a = { b: 1 }
```
This extended into import statements also. Google's styleguide is no
spacing, so we're going with that.
* chore: migrate src/helpers.ts to ESM
Doing this means we can avoid the global `types.d.ts` file and export
the interface via ESM instead.
I would ideally like to rewrite the helper module so that it doesn't
export all the functions under the `helper` namespace, but I'll leave
that for a separate PR to keep mechanical changes to one per PR and
easier to review.
* chore: migrate `src/USKeyboardLayout` to typescript
Don't think we need to expose the interface type for the keycodes so
I've left it local for now.
* retry windows unit tests
* chore: migrate `src/Connection` to TypeScript
This commit migrates `src/Connection` to TypeScript. It also changes its
exports to be ESM because TypeScript's support for exporting values to
use as types via CommonJS is poor (by design) and so rather than battle
that it made more sense to migrate the file to ESM.
The good news is that TypeScript is still outputting to `lib/` as
CommonJS, so the fact that we author in ESM is actually not a breaking
change at all.
So going forwards we will:
* migrate TS files to use ESM for importing and exporting
* continue to output to `lib/` as CommonJS
* continue to use CommonJS requires when in a `src/*.js` file
I'd also like to split `Connection.ts` into two; I think the
`CDPSession` class belongs in its own file, but I will do that in
another PR to avoid this one becoming bigger than it already is.
I also turned off `@typescript-eslint/no-use-before-define` as I don't
think it was adding value and Puppeteer's codebase seems to have a style
of declaring helper functions at the bottom which is fine by me.
Finally, I updated the DocLint tool so it knows of expected method
mismatches. It was either that or come up with a smart way to support
TypeScript generics in DocLint and given we don't want to use DocLint
that much longer that didn't feel worth it.
* Fix params being required
* chore: migrate `src/pipetransport` to typescript
Hit one bump in the fact that I want to share an interface across files.
TypeScript only lets you import/export these if you're using ESM, not
CommonJS. So the two options are:
- Migrate to ESM on a per file basis as we do this migration. This won't
affect the output as we output as CommonJS.
- Create a global `types.d.ts` file that we'll use and then migrate to
ESM after.
Right now I've gone for the second option in order to not introduce more
changes in one go. But if we end up finding we have lots of
interfaces/types/etc that we want modules to expose, we might decide
slowly introducing ESM might be a better way forwards.
* Update src/types.d.ts
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
This PR migrates the helper module to TypeScript. It's a bit of a bigger
change than others because I decided to move away from the helper class
with static methods and move towards a simpler set up where the module
is a bunch of functions. I still expose them under the `helper`
namespace to avoid this being a big change - we can update that later
when we migrate to ESM.
We do have to do some unfortunate wrangling of the promisify function.
Ideally I'd like to rely on the Node one (and the type defs) but that
doesn't work in Browserify land. I've stuck with the promisify in
`helper.ts` but pulled it into its own module to enable me to leave a
comment clarifying why we use it and the context. We can solve this with
a better web bundling story but that work is lower priority right now
than getting the `src/` directory into TypeScript.
This is a simple module but took a bit of work because:
* It wraps a Promise that can return basically anything. In a pure TS
codebase we'd solve these with generics, so you could do `new
TaskQueue<T>` where `T` will be what's returned from the queue, but
because we're calling that from JS we can't yet. I've left a TODO and
once we migrate the call sites to TS we can do a much better job than
the `void | any` type I've gone with for now.
* It was used in typedefs via `Puppeteer.TaskQueue`. I've removed that
entry from `externs.d.ts` in favour of importing it and using the type
directly. This does mean that we have imports that ESLint doesn't
realiase are actually used but I think this is better than maintaining
`externs.d.ts`.
This PR changes `src/Dialog.js` to `src/Dialog.ts` and rewrites
accordingly. Most of the changes are straight forward; the only
interesting one from a TS point of view is the `DialogType` enum. I
expose it again as `Dialog.Type` to avoid a breaking change.
This PR also exposed some bugs with our ESLint TypeScript settings and
applying the overrides, so I fixed those too.
I also updated our DocLint tool to work on TS source files over JS lib
files if they exist. This is the minimal change to keep the existing doc
system working as we're working on moving away from this system longer
term.
Fixes an edge case where Puppeteer looked for a Chromium revision when launching Firefox.
Allow appropriate Launcher to be instantiated when calling `Puppeteer.connect`.
Add an example of running Firefox.
This commit adds linting for `*.ts` files and loads up the recommended
list of TS rules from the ESLint TypeScript plugin. We can adjust the
exact rules overtime, but starting with the recommended list seems
sensible.
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
extract-zip removed support for callbacks and instead uses promises. Moreover, it has TypeScript support which allows us to remove the @types/extract-zip package.
This update allows downstream users to remove their installation of mkdirp, which uses a vulnerable version of minimist.
For more info, see https://github.com/maxogden/extract-zip/releases/tag/v2.0.0
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
Rather than use our own custom expect library, we can use expect from npm [1], which has an API almost identical to the one Puppeteer has, but with more options, better diffing, and is used by many in the community as it's the default assertions library that comes with Jest.
It's also thoroughly documented [2].
[1]: https://www.npmjs.com/package/expect
[2]: https://jestjs.io/docs/en/expect
This updates our `tsconfig.json` so it emits our JavaScript files as
well as type checking them. We compile into `./lib` which we then ship
in our npm package. The source code has moved from `./lib` into `./src`.
Because the `src/` directory is exclusively JS files, this change is a
no-op in terms of code functionality but is the first step towards being
able to replace `src/X.js` with `src/X.ts` in a way that allows us to
migrate incrementally.
The `lib` directory is gitignored, and the `src` directory is
npmignored. On `npm publish` we will now run `npm run tsc` in order to
generate the outputted code.
