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

[Feature]: Remove NodeJS runtime from package #1850

Open
Liero opened this issue Dec 3, 2021 · 18 comments
Open

[Feature]: Remove NodeJS runtime from package #1850

Liero opened this issue Dec 3, 2021 · 18 comments

Comments

@Liero
Copy link

Liero commented Dec 3, 2021

Feature request

Please, remove NodeJS runtime from a nuget package.

NodeJs could be included in separate nuget package out of convenience. Playwright could use environment variable to determine path to node binaries.

The problem

Including a NodeJS binaries slows down a CI/CD pipeline.

It has to be restored on each build (Azure Hosted pipeline).

Given that I have multiple projects referencing playwright, I have a gigabyte of unnecessary binaries that I have to upload as build artifacts and downloads in my release pipeline. My CI produces 300GB of build artifacts per months with only two developers pushing the code.

Now imagine, that other nuget references would include NodeJS runtime as well!
It has to be removed or made optional.

@pavelfeldman
Copy link
Member

pavelfeldman commented Dec 8, 2021

We consider Node.js runtime to be an implementation detail of Playwright. I'm not sure how deduping Node.js helps addressing the CI/CD challenges. We would still need Node.js on CI to run the tests.

Including a NodeJS binaries slows down a CI/CD pipeline.

My understanding is that we are talking about tens of milliseconds, not even hundreds. Given that the typical test run is minutes, this should be within the error.

It has to be restored on each build (Azure Hosted pipeline).

Could you elaborate on what is getting restored?

Given that I have multiple projects referencing playwright, I have a gigabyte of unnecessary binaries that I have to upload as build artifacts and downloads in my release pipeline. My CI produces 300GB of build artifacts per months with only two developers pushing the code.

Do I understand it right that playwright binaries are a part of the build artifacts?

More information would help us understand and prioritize this request.

@Liero
Copy link
Author

Liero commented Dec 8, 2021

We consider Node.js runtime to be an implementation detail of Playwright.

Well, Microsoft.TypeScript.MSBuild package is also dependent on NodeJS and thanks god it does not include yet another NodeJS runtime 🙄

We would still need Node.js on CI to run the tests

There is Node.js installed on all Azure hosted build agents or github actions. Otherwise you would use a docker image with Node, or a separate step to install node as part of you pipeline. Including NodeJS as part of the package is unnecessary in wast majority of cases.

Could you elaborate on what is getting restored?

Nuget package references are being restored on each CI build. If the Microsoft.Playwright package is 200MB instead of ten, it is it slower about ten seconds. Azure Hosted agent, each pipeline run does a fresh restore, so it has to be downloaded from nuget.org each time.

Do I understand it right that playwright binaries are a part of the build artifacts?

Yes. If an asp.net core project references Playwright, then the node runtime is being published with the project -> this means that output (pipeline artifacts) of my CI pipeline includes now node binaries. In my case about 1GB of node binaries since I have multiple projects referencing Playwright. This slows down my CI yet another 60 seconds.

In order to deploy the project, I need to download my build artifacts in CD pipeline. Yet another dozen of seconds.

image

Did I mentioned, that my server where the app is running already has Node runtime, so all those gigabytes are unnecessary?

You could make the path to node.exe configurable and make the node binaries optional. Maybe you could download node using Playwright.CLI just like browsers?

@pavelfeldman
Copy link
Member

  • I'm trying to understand how Node that is 50-70Mb, even if there are 3 of them becomes gigabytes
  • I'm also surprised that 200MB of nuget on Azure ends up being 10 seconds, I would imagine it to be 2

It isn't hard for us to point to a compatible Node binary, but I don't think it is going to affect your experience. From what you are reporting, we are talking about 'up to the single digit seconds' per several minutes of the test run.

@pavelfeldman
Copy link
Member

Overall I think your request is clear, so I'm marking it as collecting feedback to collect votes.

@Liero
Copy link
Author

Liero commented Dec 9, 2021

Until recently, there were 4x node runtimes (win32, win64, mac, linux) = cca 250MB, now there are 3 - 197MB. They are all being copied to output directory of each project, that references it directly or indirectly

I have following projects that I build in CI pipeline and some of them deploy in CD pipeline:

image

  1. App1 (uses playwright for PDF rendering)
  2. App1.E2ETests (uses playwright for automated ui tests)
  3. App2 (uses playwright for PDF rendering)
  4. App2.E2ETests

If I publish them I get 4x .playwright folder in the publish output.

Additionally I have following projects:

  1. App1.IntegrationTests - references App1, which copies additional .playwright folder to output directory
  2. App2.IntegrationTests - references App2, which copies additional .playwright folder to output directory

I don't need .playwright for running IntegrationTests, so I remove it after build. Although if I was about to test the PDF rendering, I would need it.

Additionally I have following have .playwright in the output directory as well, but they run only in CI pipeline, so I don't care:

  1. App1.UnitTests - references App1
  2. App2.UnitTests - references App2

@apis3445
Copy link

apis3445 commented Jan 23, 2022

For example the issue is that I publish playwright test as artifact to run the test after deploy, the node packages are 200 mb so one artifact is always 200 mb that is too much to an artifact
image
The nodejs can run from pipeline and the 200 mb is not needed. Too playwright install requires a .csproj that in publish artifact is not included, so maybe another approach can be consider for itnegrate with ci/cd as release without .csproj

@jgilbert2017
Copy link

+1
i concur, please remove the node binaries from the package and make the node executable path configurable in code. i would like to use my system installed node. this would speed up complies / nuget restore / deployment and memory footprint during runtime since the o/s would use a shared image.

@changhuixu
Copy link

image

Also, how can we only include ONLY ONE Node binary for the OS we know for sure? Currently, it defaults to include binaries for 3 OS.

@mxschmitt
Copy link
Member

In the next version only the selected platform gets included see here: #1955

We'll release it today or tomorrow.

@changhuixu
Copy link

Perfect. Thank you @mxschmitt

@jgilbert2017
Copy link

when using <PlaywrightPlatform>none</PlaywrightPlatform> how do you specify the runtime node path? thanks.

@sid-6581
Copy link

Is it just not possible to use an already installed node and not have to bundle this huge dependency with every application? Why can't "install" download the node dependency as well as the browsers?

@Liero
Copy link
Author

Liero commented Jun 23, 2022

In the next version only the selected platform gets included see here: #1955

Thanks, better than nothing, but still far from what I was hoping for :(

It's like Microsoft.EntityFrameworkCore.SqlServer package included the actual SQL Server and make it 3x for each platform!

Literally everybody who integrated Playwright into CD pipeline has the same complaint.

@304NotModified
Copy link

304NotModified commented Sep 28, 2022

FYI
This issue is nowadays the most upvoted (open) issue. (See https://github.com/microsoft/playwright-dotnet/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc)

This could be relevant as its a "P3-collecting-feedback" issue.

@joem-msft
Copy link

For now our team is working on a workaround where we're having to:

  • Exclude node executables from the build/publish outputs via custom MSBuild targets
  • rewrite and replace the playwright scripts to assume 'node' is available on the PATH. (currently in progress)

It would be nice to see this issue being resolved as it's a big blocker for adoption.

@wodyjowski
Copy link

I'm voting up on this. I actually work with a solution containing over 34 projects using Playwright.
We have 2.3GB of node.exe files 🙂
Node.js can be just checked during build...

@damianh
Copy link

damianh commented Nov 6, 2023

l I think your request is clear, so I'm marking it as collecting feedback to collect votes.

Feedback and votes have been collecting for nearly 2 years. I reckon that's been long enough.

@PhilKochan
Copy link

Yeah. Two years is a long time. But it took me an HOUR to find this Feature Request! Most devs seeing this waste of time/disk/IO would not look that long! :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests