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.
* 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
