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

Support single version principle for nuget packages. #6

Closed
AgentEnder opened this issue Mar 15, 2021 · 5 comments
Closed

Support single version principle for nuget packages. #6

AgentEnder opened this issue Mar 15, 2021 · 5 comments

Comments

@AgentEnder
Copy link
Member

Problem:

Nuget packages are managed within .csproj files, meaning different apps/libs in the same workspace might have different versions. This goes against single version principle, which Nrwl has defended many times in the Nx repo.

@AgentEnder AgentEnder added enhancement New feature or request stretch-goal labels Mar 15, 2021
@AgentEnder AgentEnder modified the milestone: Initial Release Mar 15, 2021
@AgentEnder AgentEnder added the help wanted Extra attention is needed label Apr 12, 2021
@bcallaghan-et
Copy link
Collaborator

This could be accomplished with a shared MSBuild properties file named Directory.Build.props. This file will automatically be imported into each project found below it in the file system. It may contain any MSBuild property, including <PackageReference /> elements. If the Directory.Build.props file is created next to the workspace's package.json, it will automatically apply to all current and future .NET projects. If all NuGet packages are saved there (rather than the individual project files), then it effectively functions like package.json, providing single-versioning for the workspace. The biggest drawback is that the dotnet CLI doesn't support this pattern. Any packages added through the CLI are added to the individual csproj files; updating NuGet packages would require hand-rolled XML manipulation.

@AgentEnder
Copy link
Member Author

Hmmmm, thats an interesting idea. I did not know about the directory file like that.

Like you said the dotnet cli would not install new packages to that by default though, and if anyone was to open a project in VS the package manager would not know about these packages.

Perhaps it would be better to have a section in the nx-dotnet.config.js file that has package versions listed in it, and update that section when someone uses the cli to install a package.

We would provide 2 schematics to facilitate this. One that adds the package reference to the needed csproj file, using the version from the config file if it matches or prompting a decision if it does not match. The other would be called something like sync-packages and would scan project files in the workspace and update the config file. This 2nd schematic could called at the end of the first, but would also allow it to be ran in case a dev had messed with packages from the manager

@bcallaghan-et
Copy link
Collaborator

Visual Studio 2019 recognizes packages that live in Directory.Build.props when using the visual package manager, but any changes made within that package manager are saved into the csproj file, including updates made to packages that already live in the global file.

No matter which global file ends up holding the shared packages, a schematic to rebuild the global file from the individual projects would be a nice addition. It would be useful when the projects are modified directly and when existing .NET projects are migrated into an Nx workspace. This schematic could also be used as a CI linter that ensures all packages live in the global file and there's no inconsistencies.

Between the choice of Directory.Build.props and nx-dotnet.config.js, the props file leads to less duplication. With the JS file, all packages are listed there for the benefit of the schematics, but all packages must also be listed in the csproj files for the benefit of the compiler. With the props file, all packages listed there are visible to both the schematics and the compiler (though the schematics will need XML parsing, which isn't as clean as JSON). Then, validation that the projects are in-sync is simply a check that they don't contain any package references (no need to compare version numbers).

On the other hand, using an "invisible" config file allows the schematics to only include the packages that are actually used by the individual project. The dotnet CLI will include all referenced assemblies, including those that are never used, when building or publishing an application. Setting up each csproj with only the packages it actually uses matches the pattern of publishable libraries that Nx uses with npm packages.

@AgentEnder
Copy link
Member Author

Yeah, there would definitely be more duplication in the nx-dotnet.config.js. I am still personally leaning more towards utilizing it as xml manipulation is bit more painful that just updating the js exports.

Keeping built application sizes down by dotnet not knowing about the js config file is definitely a benefit.

Either route we choose a sync-packages schematic will be needed in case changes are made inside visual studio. A more generic vs-sync schematic might be nice in case there are further things that crop up as not being cross-compatible.

There are pro's and con's to both. Here is how I see it.

nx-dotnet.config.js
    Pros:
        - Small file, easy to read at a glance
        - Easier to manipulate from a schematic
        - .NET CLI handles all XML manipulation
    Cons:
        - Does not actually affect the build
        - Visual Studio + dotnet do not know about this file.

Directory.Build.props
    Pros:
        - Packages listed here do not have to be included into project files.
        - Visual studio is somewhat aware of this file.
        - Validation of sync state is easier, do not care about the versions.
    Cons:
        - Updates made inside of VS apply to the project files, so this still has to be updated.
        - XML is harder to update
        - Packages listed here are added to the built projects, regardless of if they are used.

I am mostly leaning towards the config file, mostly due to the last con on the directory build props file. There are definitely benefits to the directory file though. The nx-dotnet schematics will have to read xml either way, but we already include xmldoc for reading these to parse dependency tree.

@AgentEnder AgentEnder self-assigned this Apr 28, 2021
@AgentEnder AgentEnder added minimum-viable-product and removed help wanted Extra attention is needed stretch-goal labels Apr 29, 2021
@AgentEnder AgentEnder moved this from To do to In progress in nx-dotnet initial release Apr 29, 2021
AgentEnder added a commit that referenced this issue Apr 29, 2021
#6 still needs to be implemented. The base sync schematic has been added, but is not implemented.
github-actions bot pushed a commit that referenced this issue Apr 29, 2021
# [0.3.0-dev.4](v0.3.0-dev.3...v0.3.0-dev.4) (2021-04-29)

### Bug Fixes

* **repo:** update .releaserc to commit package.json version back ([dea86e3](dea86e3))

### Features

* **core:** schematic for adding npm package [#5](#5) ([b97c097](b97c097)), closes [#6](#6)
AgentEnder added a commit that referenced this issue Apr 29, 2021
AgentEnder added a commit that referenced this issue Apr 29, 2021
AgentEnder added a commit that referenced this issue Apr 29, 2021
AgentEnder added a commit that referenced this issue Apr 29, 2021
AgentEnder added a commit that referenced this issue May 1, 2021
* feat(core): #6 schematic to sync nuget packages across workspace
github-actions bot pushed a commit that referenced this issue May 1, 2021
# [0.3.0-dev.5](v0.3.0-dev.4...v0.3.0-dev.5) (2021-05-01)

### Features

* **core:** support for single version principle [#6](#6) ([#32](#32)) ([81301b6](81301b6))
AgentEnder added a commit that referenced this issue May 1, 2021
#6 still needs to be implemented. The base sync schematic has been added, but is not implemented.
AgentEnder pushed a commit that referenced this issue May 1, 2021
* **repo:** update .releaserc to commit package.json version back ([dea86e3](dea86e3))

* **core:** schematic for adding npm package [#5](#5) ([b97c097](b97c097)), closes [#6](#6)
AgentEnder added a commit that referenced this issue May 1, 2021
* feat(core): #6 schematic to sync nuget packages across workspace
AgentEnder pushed a commit that referenced this issue May 1, 2021
* **core:** support for single version principle [#6](#6) ([#32](#32)) ([81301b6](81301b6))
github-actions bot pushed a commit that referenced this issue May 1, 2021
# [0.4.0](v0.3.0...v0.4.0) (2021-05-01)

### Bug Fixes

* **repo:** update .releaserc to commit package.json version back ([36d2f30](36d2f30))

### Features

* **core:** schematic for adding npm package [#5](#5) ([4f37be7](4f37be7)), closes [#6](#6)
* **core:** support for single version principle [#6](#6) ([#32](#32)) ([8e60a13](8e60a13))
@AgentEnder AgentEnder moved this from In progress to Done in nx-dotnet initial release May 1, 2021
@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

2 participants