playwright-dotnet
playwright-dotnet copied to clipboard
microsoft
Multi-target to reduce dependencies
Currently, Microsoft.Playwright has a few dependencies which are all unnecessary on modern .NET because they are part of the framework itself. This will become even more of an issue with .NET 9 since NuGet Audit will start complaining about CVEs in transitive dependencies, so all consuming modern. NET applications will get false positive warnings about e.g. a CVE in System.Text.Json .
Multi-targeting would allow to remove these dependencies on modern .NET . It would also allow to conditionally make use of newer APIs in modern .NET.
I could make a PR for this and already looked into this. Unfortunately this is not as simple as editing the csproj since quite a few new warnings are introduced in the net8.0 target which would need fixing or suppressing. So I first wanted to get some feedback on whether a PR for this would be accepted.
Which CVEs are you referring to? We aim to fix security vulnerabilities so we did it with https://github.com/microsoft/playwright-dotnet/pull/3016.
I'm referring to CVEs in general. Currently a CVE in System.Text.Json would affect both .NET Framework and modern .NET applications. With multi-targeting, only .NET Framework applications would be affected since the dependency isn't even needed on modern .NET. So something like #3016 would not be necessary in the first place (at least for modern .NET users).
Lets collect feedback for it. So far we didn't see much demand for it. Multi-targeting means having compiler directives inside the code which adds a lot of overhead for us to maintain the library, also testing wise these are then different code-paths etc.
My two cents. Some libraries have different implementations, whether you use the netstandard version or the .NET 8 version, for good or bad. The good part is that you might get a bump in performance if you use .NET-specific libraries and fewer dependencies. The bad part is that you might have different behaviors (potential bugs) on different platforms.
This would be very helpful. It would prevent modern apps from breaking on any future CVEs (it recently killed my build process)
Compiler directives should not be necessary for the case of System.Text.Json, Playwright just needs to target netstandard2.0 and net8.0 and only import STJ on .NET standard. .NET 8+ projects using Playwright will use STJ automatically, without any extra work from Playwright.
The bad part is that you might have different behaviors (potential bugs) on different platforms.
This is the case no matter what when you're targeting .NET standard unfortunately. It also doesn't apply for STJ, since you can align the version of the nuget package with .NET 8.
The only conceivable downside to this is somewhat longer build times.
Nuget's recent change to flag transitive dependencies will probably make this a more and more pressing issue for people. The three public packages that core Playwright ships with (System.Text.Json, Microsoft.Bcl.AsyncInterfaces, and System.ComponentModel.Annotations) are all polyfills which come built-in with modern .NET:
https://github.com/microsoft/playwright-dotnet/blob/8351af6cddcc38b9a0695f31a167ff7a7766a7ff/src/Playwright/Playwright.csproj#L33-L36
All of these could be removed, and PW could ship with no public dependencies
Opened PR #3080 . I know there was no clear decision to do this, but I hope the PR makes it easier to decide whether this is worth doing.
Multi-targeting means having compiler directives inside the code which adds a lot of overhead for us to maintain the library, also testing wise these are then different code-paths etc
Not necessarily? If the packages bring in the same namespaces + signatures as what the later frameworks have, then you don't need any if statements. So there codepaths stay the same.
Lets collect feedback for it. So far we didn't see much demand for it. Multi-targeting means having compiler directives inside the code which adds a lot of overhead for us to maintain the library, also testing wise these are then different code-paths etc.
I agree with @RenderMichael on this one, with Nuget's transitive dependency flag, this package is getting picked up in our vulnerability assessments, where it seems there is a potential solution to remove dependencies with Multi-targeting as per PR #3080
I also would like to second this proposal. After .NET 5, there has been this "concensus" (if you will, also somewhat addressed in this article) that, when possible, packages should try to target a netx.0 TFM to take advantage of the included runtime. If this comes with the added benefit of no more dependency issues, whether that's security or resolutions, even better. In the end, I hope there's a way to find a compromise between not adding complexity to the project and allowing this improvement to happen. Thanks!
