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

[CLEANUP] xmldoc package adds additional newlines in .csproj files #94

Open
photomoose opened this issue Aug 24, 2021 · 9 comments
Open
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request good first issue Good for newcomers scope: core

Comments

@photomoose
Copy link
Contributor

The xmldoc package adds additional newlines when manipulating the .csproj file to add the output path and msbuild task (not sure whether this affects Windows users, but definitely affects MacOS users). Although functionality is not impacted, the additional whitespace looks a bit unsightly, especially when creating xunit test projects, which bring in several PackageReferences.

For example:

<Project Sdk="Microsoft.NET.Sdk">
  
  <PropertyGroup>
    
    <TargetFramework>net5.0</TargetFramework>
    
    <IsPackable>false</IsPackable>
    
    <OutputPath>../../dist/apps/alerts-api-testing1234</OutputPath>
  </PropertyGroup>
  
  <ItemGroup>
    
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1"/>
    
    <PackageReference Include="xunit" Version="2.4.1"/>
    
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
      
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      
      <PrivateAssets>all</PrivateAssets>
      
    </PackageReference>
    
    <PackageReference Include="coverlet.collector" Version="1.3.0">
      
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      
      <PrivateAssets>all</PrivateAssets>
      
    </PackageReference>
    
  </ItemGroup>
  
  <Target Name="CheckNxModuleBoundaries" BeforeTargets="Build">
    
    <Exec Command="node ../../node_modules/@nx-dotnet/core/src/tasks/check-module-boundaries.js -p alerts-api-testing1234"/>
    
  </Target>
</Project>

The documentation for xmldoc.toString() also states "that this is for debugging only! It is not guaranteed to always output valid XML."

Is it worth considering using a different XML parsing package? I've had a little play with fast-xml-parser - it reads XML into a JS object, then converts a JS object back to XML. For example:

  // read xml into object
  let xmlDoc = parser.parse(readFileSync(projectFilePath).toString(), {
    ignoreAttributes: false,
    parseAttributeValue: true
  });

  // manipulate object
  xmlDoc.Project.PropertyGroup.OutputPath = '../../dist/apps/an-example';
  xmlDoc.Project.Target = {
    '@_Name': 'CheckNxModuleBoundaries',
    '@_BeforeTargets': 'Build',
    Exec: {
      '@_Command': 'node ../../node_modules/@nx-dotnet/core/src/tasks/check-module-boundaries.js -p an-example'
    }
  }

  let j2x = new parser.j2xParser({
    ignoreAttributes: false,
    format: true,
    supressEmptyNode: true
  });

  let newXmlDoc = j2x.parse(xmlDoc);

The contents of newXmlDoc will be similar to:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <IsPackable>false</IsPackable>
    <OutputPath>../../dist/apps/an-example</OutputPath>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1"/>
    <PackageReference Include="xunit" Version="2.4.1"/>
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
    <PackageReference Include="coverlet.collector" Version="1.3.0">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>
  <Target Name="CheckNxModuleBoundaries" BeforeTargets="Build">
    <Exec Command="node ../../node_modules/@nx-dotnet/core/src/tasks/check-module-boundaries.js -p an-example"/>
  </Target>
</Project>
@photomoose photomoose added bug Something isn't working needs-triage This issue has yet to be looked over by a core team member labels Aug 24, 2021
@AgentEnder AgentEnder added dependencies Pull requests that update a dependency file scope: core and removed needs-triage This issue has yet to be looked over by a core team member labels Aug 24, 2021
@AgentEnder
Copy link
Member

Hey @photomoose, I had not actually seen that the xmldoc toString method is not a guarantee. That's a shame, I would think it would be a main goal of an xml library.

The code looks much simpler with fast-xml-parser anyawys, and you've already got it sketched up. If you want to put in a PR for it go ahead, there are a few tests that should be updated as well.

@AgentEnder
Copy link
Member

If you do decide to work on it, go ahead and assign this issue to yourself.

@photomoose
Copy link
Contributor Author

Sure, I'll see what I can do. I don't appear to have permissions to assign this issue to myself, however - could you assign it to me, please?

@AgentEnder
Copy link
Member

Ah, my bad, I thought you could 😄. You are assigned.

Thanks for the contributions!

@AgentEnder
Copy link
Member

@photomoose Are you still planning to work on this? If not, I may pick it up.

@photomoose
Copy link
Contributor Author

photomoose commented Sep 15, 2021 via email

@AgentEnder
Copy link
Member

@photomoose I'm going to tackle this this weekend, your initial investigations should provide a good starting point. Thanks!

@AgentEnder AgentEnder assigned AgentEnder and unassigned photomoose Oct 14, 2021
@photomoose
Copy link
Contributor Author

@AgentEnder great stuff, I look forward to the update!

FYI: we now have 104 C# projects in our monorepo! We use Rider as our IDE and have imported all the projects into a single solution file, however we use NX Console in VS Code to create the projects via nx-dotnet.

@AgentEnder AgentEnder changed the title [BUG] xmldoc package adds additional newlines in .csproj files [CLEANUP] xmldoc package adds additional newlines in .csproj files Dec 20, 2021
@AgentEnder AgentEnder removed their assignment Dec 20, 2021
@AgentEnder AgentEnder added good first issue Good for newcomers enhancement New feature or request and removed bug Something isn't working labels Dec 20, 2021
@khalilou88
Copy link

Have you tried to add this options xmldoc.toString({ compressed: true, preserveWhitespace: true }). More information here nfarina/xmldoc#60.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request good first issue Good for newcomers scope: core
Projects
None yet
Development

No branches or pull requests

3 participants