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

considere packing the targets as buildTransitive too #237

Open
Doraku opened this issue Oct 10, 2024 · 2 comments
Open

considere packing the targets as buildTransitive too #237

Doraku opened this issue Oct 10, 2024 · 2 comments

Comments

@Doraku
Copy link

Doraku commented Oct 10, 2024

First let me preface this by thanking you for this awesome work. I was somewhat doing a much poorer implementation of your solution before I found your package, deleting all that ugly stuff was a relief ❤️

Is the feature request related to a problem

I'm making a package to ease up my configuration across for all my project, much like your ProjectDefaults and I want to include a reference to Polyfill. While this project is a development dependency that is not supposed to flow as an indirect reference, it is actually possible to include it by removing the PrivateAssets="all" of the PackageReference. I'm doing this to automatically include a collection of analyzer packages for exemple.

Describe the solution

The issue is that the targets file of Polyfill does not flow as an indirect dependencies because it's not present in a buildTransitive folder of the nuget (more info here) just in case). Doing so, while kind of unnatural for a development dependency (hence why I am requesting your opinion here), is quite an easy fix by adding <file src="Polyfill.targets" target="buildTransitive"/> to the nuspec.

Describe alternatives considered

Currently I am repacking your code files into my own package like so

  <ItemGroup>

    <PackageReference Include="Polyfill" Version="7.0.0" PrivateAssets="all" GeneratePathProperty="true" />

    <!-- fix for Polyfill transitive reference -->
    <None Include="$(PkgPolyfill)\build\Polyfill.targets" Pack="true" PackagePath="projects" Visible="false" />
    <Compile Update="$(PkgPolyfill)\contentFiles\**\*" Pack="true" PackagePath="contentFiles\cs\netstandard2.0\Polyfill\" Visible="false" />

  </ItemGroup>

and in the targets file of my own package

  <!-- fix for Polyfill transitive reference -->
  <Import Project="$(MSBuildThisFileDirectory)Polyfill.targets" Condition="Exists('$(MSBuildThisFileDirectory)Polyfill.targets')"/>
  <ItemGroup>
    <Compile Update="@(Compile)">
      <Visible Condition="'%(NuGetItemType)' == 'Compile' and '%(NuGetPackageId)' == 'DefaultCSharp'">false</Visible>
    </Compile>
  </ItemGroup>

This works fine but obviously I would prefer for your project to be a proper dependency instead of hiding it like so, more people who are stuck supporting multiple targets should know about it :).

Additional context

My full project is here for completeness, again thank you for this.

@SimonCropp
Copy link
Owner

wouldnt this change the default behavior of polyfill? ie that we dont want to to flow through dependencies?

@Doraku
Copy link
Author

Doraku commented Oct 17, 2024

Since your package is declared as a development dependency it should be added by default with PrivateAssets="All" which prevent its transitive inclusion. You have to explicitly remove that for the reference to flow, which nobody should do (except me because I like going against the flow).

Again my usage is definitely not conventional so I would totally understand you not making this change.

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

No branches or pull requests

2 participants