playwright-dotnet icon indicating copy to clipboard operation
playwright-dotnet copied to clipboard

[Feature]: Remove NodeJS runtime from package

Open Liero opened this issue 4 years ago • 29 comments

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.

Liero avatar Dec 03 '21 08:12 Liero

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.

pavelfeldman avatar Dec 08 '21 06:12 pavelfeldman

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?

Liero avatar Dec 08 '21 14:12 Liero

  • 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 avatar Dec 08 '21 21:12 pavelfeldman

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

pavelfeldman avatar Dec 08 '21 21:12 pavelfeldman

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

Liero avatar Dec 09 '21 08:12 Liero

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

apis3445 avatar Jan 23 '22 01:01 apis3445

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

jgilbert2017 avatar Feb 10 '22 14:02 jgilbert2017

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.

changhuixu avatar Feb 14 '22 17:02 changhuixu

In the next version only the selected platform gets included see here: https://github.com/microsoft/playwright-dotnet/pull/1955

We'll release it today or tomorrow.

mxschmitt avatar Feb 14 '22 18:02 mxschmitt

Perfect. Thank you @mxschmitt

changhuixu avatar Feb 14 '22 18:02 changhuixu

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

jgilbert2017 avatar Feb 14 '22 19:02 jgilbert2017

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?

sid-6581 avatar Jun 22 '22 18:06 sid-6581

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.

Liero avatar Jun 23 '22 09:06 Liero

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.

304NotModified avatar Sep 28 '22 23:09 304NotModified

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.

joem-msft avatar May 19 '23 18:05 joem-msft

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

wodyjowski avatar Sep 22 '23 17:09 wodyjowski

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.

damianh avatar Nov 06 '23 12:11 damianh

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! :(

PhilKochan avatar Dec 11 '23 00:12 PhilKochan