Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat) Add option to fetch Firefox Nightly #5467

Merged
merged 9 commits into from Mar 10, 2020
Merged

Conversation

@mjzffr
Copy link
Collaborator

@mjzffr mjzffr commented Feb 28, 2020

Add Firefox support to BrowserFetcher and the install script.
By default, the latest Firefox Nightly is downloaded
directly from archive.mozilla.org (dmg, tar.bz2 and zip)

This also required changes that impact puppeteer.launch()
and puppeteer.executablePath()

Fixes #5151

Add Firefox support to BrowserFetcher and the install script.
By default, the latest Firefox Nightly is downloaded
directly from archive.mozilla.org (dmg, tar.bz2 and zip)

This also required changes that impact `puppeteer.launch()`
and `puppeteer.executablePath()`

Fixes #5151
@mjzffr mjzffr force-pushed the mjzffr:fetchNightly branch from 2daac67 to fb5e25c Feb 28, 2020
@@ -37,13 +37,13 @@ npm i puppeteer
# or "yarn add puppeteer"
```

Note: When you install Puppeteer, it downloads a recent version of Chromium (~170MB Mac, ~282MB Linux, ~280MB Win) that is guaranteed to work with the API. To skip the download, see [Environment variables](https://github.com/puppeteer/puppeteer/blob/v2.1.1/docs/api.md#environment-variables).
Note: When you install Puppeteer, it downloads a recent version of Chromium (~170MB Mac, ~282MB Linux, ~280MB Win) that is guaranteed to work with the API. To skip the download, or to download a different browser, see [Environment variables](https://github.com/puppeteer/puppeteer/blob/v2.1.1/docs/api.md#environment-variables).

This comment has been minimized.

@mathiasbynens

mathiasbynens Mar 1, 2020
Member

For the record, we probably don’t want to do this right now, but in the future we could work towards npm install puppeteer fetching both Chromium and Firefox binaries by default. I like your approach of taking one step at a time 👍🏻

docs/api.md Outdated Show resolved Hide resolved
@mathiasbynens
Copy link
Member

@mathiasbynens mathiasbynens commented Mar 1, 2020

I’ll take a closer look during the week, but just wanted to drop a note saying I’m super excited about this!

@mathiasbynens mathiasbynens requested a review from hanselfmu Mar 1, 2020
docs/api.md Outdated Show resolved Hide resolved
Copy link
Member

@mathiasbynens mathiasbynens left a comment

Generally looks good, just some questions and suggestions. @hanselfmu could you PTAL as well, especially w.r.t. dropping Node.js v8?

docs/api.md Outdated Show resolved Hide resolved
install.js Outdated Show resolved Hide resolved
}).on('error', e => reject(e));

function parseVersion() {
const regex = /firefox\-(?<version>\d\d)\..*/gm;

This comment has been minimized.

@mathiasbynens

mathiasbynens Mar 2, 2020
Member

Loving the use of named capture groups here!

This will fail on Node.js v8.x.x so we'd want to land ##5365 first. @hanselfmu could you please take another look?

This comment has been minimized.

@hanselfmu

hanselfmu Mar 6, 2020
Collaborator

The PR to drop Node.js v8.x support is labeled as a breaking change, which means landing it will be in Puppeteer v3. I think we have two options now:

  1. land both of them in v3;
  2. temporarily use Node.js-8-compatible code for now, and when v3 lands, update the legacy code.
    @mjzffr what do you think? Do you have other ideas?

This comment has been minimized.

@mathiasbynens

mathiasbynens Mar 9, 2020
Member

IMHO major version bumps are no big deal. We shouldn't hold back on publishing v3.

This comment has been minimized.

@hanselfmu

hanselfmu Mar 10, 2020
Collaborator

Sounds good. I'll publish v3 now and include this PR.

This comment has been minimized.

@nylen

nylen Mar 16, 2020

@hanselfmu @mathiasbynens What happened with this? This PR doesn't seem to be available in a release version yet.

install.js Outdated Show resolved Hide resolved
test/launcher.spec.js Outdated Show resolved Hide resolved
mjzffr and others added 8 commits Mar 2, 2020
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
@mjzffr
Copy link
Collaborator Author

@mjzffr mjzffr commented Mar 9, 2020

Copy link
Collaborator

@hanselfmu hanselfmu left a comment

LGTM

@hanselfmu hanselfmu merged commit 33f1967 into puppeteer:master Mar 10, 2020
1 of 7 checks passed
1 of 7 checks passed
@cirrus-ci
Chromium (node10 + linux) Task Summary
Details
@cirrus-ci
Chromium (node12 + linux) Task Summary
Details
@cirrus-ci
Chromium (node8 + linux) Task Summary
Details
@cirrus-ci
Chromium (node8 + macOS) Task Summary
Details
@travis-ci
Travis CI - Pull Request Build Failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@googlebot
cla/google All necessary CLAs are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants