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: download ffmpeg during install step #5249

Conversation

aslushnikov
Copy link
Collaborator

This patch starts downloading FFMPEG like we download our browsers
instead of bundling it in the NPM package.

With this patch, NPM size is reduced from 8.8MB to 1.7MB.

Consequences:

  • npx playwright is drastically faster now
  • playwright driver for language bindings is way smaller
  • projects that bundle Playwright can pass Apple Notorization

Fixes #5193

@aslushnikov aslushnikov changed the title devops: download ffmpeg during install step feat: download ffmpeg during install step Feb 2, 2021
Copy link
Contributor

@dgozman dgozman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look interesting!

super(packagePath, browser, playwrightOptions);

const browsersPath = browserPaths.browsersPath(packagePath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put everything we need in PlaywrightOptions or somewhere close, and introduce a helper method that will retrieve all the paths on demand? I feel like we have too much juggling with browserPaths scattered across the codebase.

@@ -95,6 +96,20 @@ function getDownloadUrl(browserName: BrowserName, revision: number, platform: Br
['win64', '%s/builds/webkit/%s/webkit-win64.zip'],
]).get(platform);
}

if (browserName === 'ffmpeg') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we now have to compress our "downloading foo..." spam into a single "downloading..." item.

This patch starts downloading FFMPEG like we download our browsers
instead of bundling it in the NPM package.

With this patch, NPM size is reduced from 8.8MB to 1.7MB.

Consequences:
- `npx playwright` is drastically faster now
- playwright driver for language bindings is way smaller
- projects that bundle Playwright can pass Apple Notorization

Fixes microsoft#5193
@aslushnikov aslushnikov force-pushed the download-ffmpeg-during-install-step branch from a9cb5cf to be7f2ef Compare February 3, 2021 17:00
@aslushnikov aslushnikov merged commit cb1b642 into microsoft:master Feb 3, 2021
@aslushnikov aslushnikov deleted the download-ffmpeg-during-install-step branch February 3, 2021 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] macOS notarization fails
2 participants