* chore: update how we track coverage during unit tests
The old method of tracking coverage was causing issues. If a test failed
on CI, that test's failure would be lost because the test failing would
in turn cause the coverage to fail, but the `process.exit(1)` in the
coverage code caused Mocha to not output anything useful.
Instead the coverage checker now:
* tracks the coverage in memory in a Map (this hasn't changed)
* after all tests, writes that to disk in test/coverage.json (which is
gitignored)
* we then run a single Mocha test that asserts every method was called.
This means if the test run fails, the build will fail and give the error
about that test run, and that output won't be lost when the coverage
then fails too.
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
* chore: enforce src/protocol.d.ts is in sync
On CI we run `npm run compare-protocol-d-ts` which checks that the file
on disk is up to date with the protocol we fetch from the browser.
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
* Increased the timeout to a flat 25 second for every build because we
still see the odd, non-reproducible timeout on a variety of machines.
* Removed an extraneous `npm run test-install` which meant we did that
check twice on each CI run.
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.
We don't support it and v3 shipped without including puppeteer-web in the browser. People are welcome to manually use Browserify to try to get Puppeteer running in a browser but it ultimately isn't our primary focus right now.
Getting puppeteer-core able to run in a browser is something we'll be looking at in the future so we'll revisit this soon.
* 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: add test for npm package installing correctly
This command packs up the module and installs it again to check we're
correctly bundling everything we need to allow users to do a fresh
install.
* install realpath
* 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.
