This CL migrates all the tests to TypeScript. The main benefits of this is that we start consuming our TypeScript definitions and therefore find errors in them. The act of migrating found some bugs in our definitions and now we can be sure to avoid them going forwards.
You'll notice the addition of some `TODO`s in the code; I didn't want this CL to get any bigger than it already is but I intend to follow those up once this lands. It's mostly figuring out how to extend the `expect` types with our `toBeGolden` helpers and some other slight confusions with types that the tests exposed.
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
It was causing some infra issues when trying to migrate tests to
TypeScript (that's WIP in another branch that I'll have up soon). It's
unusual to have the entire src in TS except for the main file, which
then reaches into the compiled `lib` directory for the files it needs.
Much better is to move the entry point into TypeScript itself and update
the `main` entry in our `package.json` to point to the compiled output.
This also has the advantange of hooking up all the TS type defs that we
are shipping and will make that process easier too, along with making it
easier to port our tests to TypeScript.
These files will be used by both the web and node versions of Puppeteer.
Another name for this might be "core" but I don't want to cause
confusion with the puppeteer-core package that we publish at the moment.
This is another step towards making Puppeteer agnostic of environment
and being able to run in Node or a browser.
The files in the `node` directory are ones that would only be needed in
the Node build - e.g. the code that downloads and launches a local
browser instance.
The long term vision here is to have three folders:
* node - Node only code
* web - Web only code
* common - code that is shared
But rather than do that in one PR I'm going to split it up to make it
easier to review and deal with.
The TypeScript definition erroneously made `options` required. We can
fix it by providing a default value, which means users calling the
function will be able to leave it blank without TS complaining.
Issues like this are a +1 to porting our tests to TypeScript in order to
catch these on our own test suite, so that's something we should look into.
* chore: create new Debug module
This debug module can be used in either Node or the browser. We'll use
the `debug` module in Node land, but fallback to a simple `console.log`
solution when in the browser in an attempt to keep our browser bundle
size down.
* Use our debug wrapper rather than Node's `debug`.
A lot of the helpers in `helpers.ts` are heavily bound to NodeJS and at
the moment we're trying to make the `Connection` class be able to run in
multiple environments. Its only remaining Node dependency was its
reliance on `helpers.ts`, which it only needed for `assert`.
This is a useful change also because `helpers.ts` is quite large and
full of functions that do different things; I think we can name them
better and move them into modules with a specific purpose rather than a
generic `"helpers"` dumping ground.
Once this change lands `Connection` should be usable in the browser.
Fix child process killing when the parent process SIGINTs.
If you `ctrl + c` the Puppeteer parent process, we would sometimes not properly handle killing of the child processes. This would then leave child processes behind, with running Chromium instances. This in turn could block Puppeteer from launching again and results in
cryptic errors.
Instead of using the generic `process.kill` with the process id (which for some reason is negative the pid, which I don't get), we can kill the child process directly by calling `proc.kill`.
Fixes#5729.
Fixes#4796.
Fixes#4963.
Fixes#4333.
Fixes#1825.
This PR splits the logging for send and receive messages in to separate debug channels.
This way it is easier to filter and takes advantage of debug's automated coloring to make
it easier to visually parse on the command line
* chore: remove "Extracting..." log message
Fixes#5741.
* test: support extra Launcher options and skips
The extra Launcher options and skipping conditions enable
unit tests to be run more easily by third-parties, e.g.
browser vendors that are interested in Puppeteer support.
Extra Launcher options were previously removed as part of
switching away from the custom test harness.
* test: enable more tests for Firefox
* chore: move `index.js` into `src`
This is the first part of a series of pull requests that will slowly
make it possible to initialise Puppeteer for either a Node environment
or a web one. By creating the `initialisePuppeteer` function we can
inject dependencies in for the given version of Puppeteer (e.g. inject a
different debug library for Node vs the web).
This PR moves the initialisation into `src` and calls into it from the
root `index.js`.
This PR starts exploring the Page class and how to best document it. It explores how best to document events in the system, and I think pulling them out into an `enum` is the best solution here. It lets us end up with a page of docs that explicitly lists all the events the page class can ever emit.
Just one was used externally and I wrapped that up in a method. I think
it's a useful method to provide (I can imagine wanting to know if JS is
enabled on a page) so I think there's no harm here (I'd rather that then
have JSHandle reach into a private variable).
Replacing the Node EventEmitter with Mitt caused more problems than
anticipated for end users due to the API differences and the amount of
people who relied on the EventEmitter API. In hindsight this clearly
should have been explored more and then released as a breaking v4.
This commit rolls us back to the built in Node EventEmitter library
which we can release to get everyone back on stable builds. We can then
consider our approach to migrating to Mitt and when we do do that we can
release it as a breaking change and properly document the migration
strategy and approach.
* chore: migrate to Mitt as the EventEmitter
This commit moves us to using Mitt [1] for the event emitter in
Puppeteer. This removes our dependency to Node's EventEmitter which is
part of a larger stream of work to enable a Puppeteer-web version that
doesn't depend on Node.
There are no large breaking changes as we support the main methods that
EventEmitter had, but it also provides some methods that Puppeteer
didn't use. Technically end users could depend on this but it's
unlikely.
[1]: https://github.com/developit/mitt
It conflicts with an inbuilt TypeScript `Request` type so can cause
confusion when in TS land. Note: `Response.ts` and `Worker.ts` also
suffer from this; PRs to rename them are incoming.
* Don't use expect within Promises (#5466)
If a call to expect fails within a Promise it will not
be resolved, and causing the test to crash.
The patch aligns the code similar to what is used by all
the other tests.
We should import them just like any other module. This commit makes that
change. It does not change any behaviours or the types themselves.
EXPECTED_PROTOCOL_DIFF as we're updating the structure of it.
This corresponds to Chromium 83.0.4103.0.
This roll includes:
- Enable SameSiteByDefaultCookies and CookiesWithoutSameSiteMustBeSecure https://crrev.com/c/2122809
* 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.
