QRCoder icon indicating copy to clipboard operation
QRCoder copied to clipboard

Dependencies

Open generateui opened this issue 2 years ago • 1 comments

From ./Readme.md:

It hasn't any dependencies to other libraries

In ./QRCoder/QRCoder.csproj:

  <ItemGroup Condition=" '$(TargetFramework)' == 'net35' or '$(TargetFramework)' == 'net40' ">
    <Reference Include="PresentationCore" />
    <Reference Include="PresentationFramework" />
    <Reference Include="WindowsBase" />
  </ItemGroup>

  <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' or '$(TargetFramework)' == 'netstandard2.0' ">
    <PackageReference Include="System.Text.Encoding.CodePages" Version="5.0.0" />
  </ItemGroup>

  <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'net5.0' or '$(TargetFramework)' == 'net5.0-windows' ">
    <PackageReference Include="System.Drawing.Common" Version="5.0.3" />
  </ItemGroup>
	<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0-windows' ">
		<PackageReference Include="System.Drawing.Common" Version="6.0.0" />
	</ItemGroup>

I was surprised, but I can totally see that above dependencies with their conditions don't count "as much". I doubt the label "no dependencies to other libraries" passes though, so maybe it's good to slightly rephrase that second sentence of ./readme.md to not have newcomers lose (some) trust (tbh it immediately did when I looked).

Very nice work though other than that.

generateui avatar Oct 31 '22 13:10 generateui

It doesn't have external library dependencies is what he means, I noticed from Net5+ a lot of built-in namespaces are extracted to nuget packages - so I get why it says no dependencies on libraries, it's also dependency on RunTime -> see System.*

ThaDaVos avatar Jan 27 '23 14:01 ThaDaVos

Hi @generateui ,

thanks for your question. The line from the readme.md you are referring to, is still from the early days. Back in 2013 (if I remember correctly my first release targeted .NET 3.5) the library had no dependencies at all.

Whether this claim is still correct today is a matter of debate. As @ThaDaVos has pointed out, the dependencies are essentially packages from the System.* namespace, which Microsoft has gradually outsourced to its own packages over the years. In my opinion, they are part of the .NET (core) framework and therefore not "external" dependencies (as a 3rd-party graphics library would be, for example).

However, I can still see the idea that prompted you to open the issue and am open to suggestions on how to formulate this better.

codebude avatar Apr 20 '24 21:04 codebude

I'm a collaborator/maintainer at GraphQL.NET and linq2db, and in both scenarios there is a goal for the projects to have "no external dependencies" - and it is so for .NET 6+. However, some MS packages are necessary to utilize modern technologies like ValueTask and Span with older platforms. So it ends up that Microsoft.Bcl.AsyncInterfaces, System.Memory, System.Text.Json and other MS NuGet packages become dependencies when targeting .NET Standard and other older platforms. Even so, I would consider these projects to have "no external dependencies", as really the only dependences exist for backwards compatibility scenarios.

Here, in a similar fashion, I would consider System.Drawing.Common to part of .NET proper, even though it is distributed separately, and consider this project to have "no external dependencies" even if it does depend on System.Drawing.Common. It is also still true that there are no dependencies when used with .NET Framework. Regardless, any dependency increases the size of the application, and it would be better if it truly had no dependencies.

However, I believe a bigger issue is that the exposed API changes based on the targeted framework. So if a NuGet library targets .NET Standard 2.0 and depends on QRCoder, but the end user compiles their application to target .NET 6, the application breaks with "MethodNotFoundException" or similar because the method referenced by the middle library does not exist when compiled against a "net6.0" TFM - even when run on Windows! This is very confusing for developers. By contrast, System.Drawing.Common targets all TFMs but throws an exception when executed on Linux. This exception is well documented across the internet and not specific to QRCoder.

I suggest breaking QRCoder into 4 or more packages for the next major version (2.0.0) as follows:

  • QRCoder
  • QRCoder.SystemDrawingCommon
  • QRCoder.SkiaSharp
  • QRCoder.ImageSharp

Then any developer can select which additional dependency, if any, they wish to use within their application, based on what platform, licensing or other restrictions they may have.

On a side note, some libraries include two packages for the ImageSharp dependency - one for v2, and one for v3+, due to the licensing changes within ImageSharp. I suggest targeting v2 and adding tests to ensure it works when v3 is referenced by the user.

Shane32 avatar Apr 21 '24 00:04 Shane32

@Shane32 this sounds really nice. I like the idea of splitting it this way. I will create an issue for discussing all things around release 2.0 (how and what to split?, how to handle the wiki for multiple repositories?, Supported platform tags, ...) after 1.5.0 was finished because there are multiple PRs and ideas for 2.0 that should be streamlined before investing more thoughts and work into a 2.0 release.

codebude avatar Apr 21 '24 15:04 codebude

@generateui I re-wrote the dependencies part in the readme: https://github.com/codebude/QRCoder/pull/507/files

Hope this helps to avoid confusion in the future.

codebude avatar Apr 27 '24 20:04 codebude