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.
* chore: Add Windows to Travis
This commit runs the unit tests on Windows.
There are two tests failing on Windows that we skip.
I spoke to Mathias B and we agreed to
defer debugging this for now in favour of getting tests running on
Windows. But we didn't want to ignore it forever, hence giving the test
a date at which it will start to fail.
This was missed in the Mocha migration.
The options I've removed from the docs were removed from the code; I didn't think it worth implementing them in Mocha land until we definitely needed them. So shout if you miss any of the options!
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
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`.
Rather than a denylist (`.npmignore`) we can instead use an allowlist
via the `files` option in `package.json`. This makes it much harder to
accidentally include files or folders in the build as you have to
explicitly list the files that will be included.
Fixes#5648.
The change to the install script to require TypeScript works fine when
installing from npm (because on npm the `lib` directory with the
compiled code already exists) but doesn't if you install from a GitHub
URL. By default it seems npm uses the `files` list when you install from
GitHub which means it's missing a bunch of files that we need to
compile.
Additionally by default when installing from a GitHub URL npm doesn't
install the dependencies which is an issue for us when we need to
compile TypeScript.
The fix is to create a `prepare` script that runs TypeScript if
required. From the npm docs [1]:
> `prepare`: Run both BEFORE the package is packed and published, on
> local npm install without any arguments, and when installing git
> dependencies
And from the npm docs on install [2], it confirms that if a package has
a `prepare` script it is run when installing from GitHub:
> As with regular git dependencies, dependencies and devDependencies
> will be installed if the package has a prepare script, before the
> package is done installing.
Despite having the `prepare` script we still need the TypeScript check
in `install.js` to satisfy the 3rd scenario below where we need to force
a compile:
* If I'm a user installing `puppeteer@X` from npm, the module is
published with the `lib/` directory of compiled code, so I'm set.
* If I'm a user installing Puppeteer from GitHub, the `prepare` script
will run TypeScript for me so I'm set.
* If I'm a developer working on Puppeteer, the `prepare` script also
runs but _after_ `npm install` which means `install.js` fails as it
requires `./lib/helper.js`. So in `install.js` we call
`compileTypeScriptIfRequired` to catch this case.
[1]: https://docs.npmjs.com/misc/scripts
[2]: https://docs.npmjs.com/cli/install
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
Fixes#5660.
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.
I lost some time debugging before realising that I needed to run tsc. I
don't really want to put `npm run tsc` before this command else we'll
run tsc multiple times on each CI build, so I think this message is
suitable.
Travis defines `process.env.TRAVIS` and if that exists we don't want to
log this as on CI we're guaranteed to have an up to date `lib/`
directory.
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>
This commit updates all the non-Puppeteer unit tests to run using Mocha and then deletes the custom test runner framework from this repository. The documentation has also been updated.
Rather than maintain our own test runner we should instead lean on the community and use Mocha which is very popular and also our test runner of choice in DevTools too.
Note that this commit doesn't remove the TestRunner source as it's still used for other unit tests, but they will be updated in a future PR and then we can remove the TestRunner.
The main bulk of this PR is updating the tests as the old TestRunner passed in contextual data via the `it` function callback whereas Mocha does not, so we introduce some helpers for the tests to make it easier.
Our CI build has been incredibly flaky across all three of our current CIs:
* Appveyor: Chromium + Windows
* Travis: Firefox + Linux, Chromium + Linux
* Cirrus: Chromium + Linux, Chromium + Mac
Legitimate issues and errors have been missed because it's expected that the CI is red and therefore it's not seen as an issue when a PR's build fails.
We should have a build that covers the full combo of browsers and operating systems but it's more important to have a consistent, reliable green build where failures are genuine. So this commit strips our CI back to Chromium on Linux on Travis, and nothing more. Once this is stable we will expand out into more operating systems and bring back Firefox, too.
Our logic around missing methods wasn't quite right; if there is no set of missing methods for a class it _is_ an error and we still need to report it, we don't want to `continue`.
See the large code comment in the diff for a full explanation but we can't rely on the functions being referentially equivalent so instead we test the behaviour in duplicate tests across the deprecated method and the new method.
This is expected as we now alias `emulateMedia` in `index.js` which isn't a file checked by DocLint. We alias there to avoid having the function overriden by the `asyncInstallHooks` code.
This commit updates doclint to know about methods that we expect it will find are missing and in that case just skip over them. We should only do this for methods where we plan to deprecate them or we have to define them in an odd way to work around some problem (and if that's the case long term we should fix that problem so we can define them as normal).
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
They fail because cookies in Firefox return a `sameSite` key which the tests don't expect.
This is a solution that at least gets the Travis Firefox build (hopefully!) green again. Longer term it'd be great to allow the assertion to change based on the browser, rather than skip these tests entirely.
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.
TypeScript seems to struggle to understand `Promise.all` when the items in the array return different types. If we were authoring in TS we could fix this with TS generics (`Promise.all<OurTypeHere>(...)`) but for now we can typecast the result. We'll fix this properly when we author in TS.
