RFC: Improve NuGet Package Structure & Dependencies

[mitchelsellers]

Description of Problem

Currently, there are a number of NuGet packages that provide DotNetNuke dependencies for consumption by third-parties. These packages do work, and are used to a great extent, but don't always provide the best experience if users are not aware of what they do.

After reviewing with @donker we are proposing the following changes.

Proposed Change 1: Updating the Description of packages

Package	New Description
DotNetNuke.Core	Provides basic references to the DotNetNuke.dll to develop extensions for the DNN Platform
DotNetNuke.Instrumentation	Provides references to enhanced logging & instrumentation available within DNN Platform for extension developers. Including access to Log4Net
DotNetNuke.Web	Provides references to core components such as Caching, Security and other security-related items
DotNetNuke.Web.Client	Provides API references for usage of the Client Dependency Framework (CDF) for the inclusion of CSS & JS files within DNN Platform
DotNetNuke.Web.Deprecated	Provides API references for deprecated items removed from the primary API's, such as DNN's Telerik components
DotNetNuke.WebApi	Provides API references to build Web API endpoints within the DNN Platform repository
DotNetNuke.Providers.FolderProvider	Provides API references needed to develop custom folder providers, or to consume folder providers
DotNetNuke.Web.MVC	Provides API references needed to develop MVC extensions within DNN Platform

Proposed Change 2: No longer publish non-recommended packages

To help prevent individuals from adding more references than are needed, the DotNetNuke.Bundle package should no longer be published.

Proposed Change 3: Improve DNN Package to DNN Package references

To aid in development, dependencies should be set on packages to auto-install related packages.

Package	New DNN Dependencies
DotNetNuke.Instrumentation	DotNetNuke.Core
DotNetNuke.Web	DotNetNuke.Core
DotNetNuke.Web.Client	DotNetNuke.Core
DotNetNuke.Providers.FolderProfiler	DotNetNuke.Core
DotNetNuke.Web.MVC	DotNetNuke.Core, DotNetNuke.Web.Client, DotNetNuke.WebApi

Proposed Change 4: Improve Third-Party Dependency Management

Again to further improve

Package	New Dependencies
DotNetNuke.WebApi	Json.NET
DotNetNuke.Web.MVC	Microsoft.AspNet.Mvc, Microsoft.AspNet.WebApi.Core, Microsoft.AspNetRazor, Microsoft.AspNet.WebPages, Microsoft.Web.Infrastructure

Decision Point & Question

Decision Point 1: Changes

Do all of these changes make sense? Any adjustments

Decition Point 2: Backward Compatibility

Is this something that should result in a change to existing packages? Or only for 9.3.1/9.4.0 and later?

Alternatives Researched

Items could be left as is, and manual references can be added

Affected version

All package versions

[bdukes]

What's the problem with providing DotNetNuke.Bundle? It's not going to hurt anything to reference more components (they're already in DNN itself, it's not adding to the application). I do think it's a little confusing, but it would be good to have a package just named DotNetNuke that has depends on all packages someone would want to write a DNN extension, so they don't have to fund the right combination of packages.

Similarly, can we combine any of these existing packages? Are they all useful (e.g. does anyone want DotNetNuke.Core without DotNetNuke.Instrumentation; does anyone depend just on DotNetNuke.Web without also depending on some other common package)?

[mitchelsellers]

@bdukes I don't think there would be any real harm in Combining DotNetNuke.Core, DotNetNuke.Instrumentation and DotNetNuke.Web

[donker]

IMHO the DotNetNuke.Web.Mvc package suggests it is just a dll (because we have a dll under that name). The guiding principle should be that the name and description of bundles is clear and focuses on a common use case. I.e. developing an MVC module. "Bundle" is a misnomer IMO as it does not say what the bundle is for.

I distinguish two types of packages:

1. A (small) siloed package of one or more dlls. E.g. the Core and Instrumentation packages. The former is used in all larger packages. The latter is used by someone in a project with an advanced need. Naming can remain as is, IMHO.
2. A (larger) bundled package. This targets a use case as described above. These should start with a conventional name so it is clear. E.g. DotNetNuke.Dev or DotNetNuke.Development. Then DotNetNuke.Development.Mvc is the package meant for MVC module developers.

My suggestions:

Package	Dependencies/Action
DotNetNuke.Bundle	Delete
DotNetNuke.Core	DotNetNuke.dll Microsoft.ApplicationBlocks.Data.dll
DotNetNuke.Instrumentation	DotNetNuke.Core DotNetNuke.Instrumentation.dll DotNetNuke.Log4Net.dll
DotNetNuke.Providers.FolderProviders	DotNetNuke.Core DotNetNuke.Providers.FolderProviders.dll
DotNetNuke.SiteExportImport	Delete
DotNetNuke.Web.Client	DotNetNuke.Core DotNetNuke.Web.Client.dll
DotNetNuke.Web.Deprecated	DotNetNuke.Web.Deprecated.dll Telerik.Web.UI.dll
DotNetNuke.Web.Mvc	DotNetNuke.Web DotNetNuke.Web.Mvc.dll
DotNetNuke.Web	DotNetNuke.Core DotNetNuke.Web.dll DotNetNuke.WebUtility.dll
DotNetNuke.WebApi	Delete (Replace)

New Packages:

Package	Dependencies
DotNetNuke.Development.WebApi	DotNetNuke.Web Microsoft.AspNet.WebApi.Client System.Web.Http Newtonsoft.Json
DotNetNuke.Development.Mvc	DotNetNuke.Development.WebApi Microsoft.AspNet.Mvc Microsoft.AspNet.Razor Microsoft.AspNet.WebPages

I'll also do a quick poll to see if the latter package is really complete for most MVC style modules.

Finally: I began this because I felt the need for a DotNetNuke.Development.Everything package. A package that would include all dlls with their correct version numbers for a particular version of DNN. The reason being that if you need to target let's say DNN 8, you'd be left researching which version of a 3rd party dll was shipped with this. It is a reason for developers to shun NuGet in favor of referencing dlls directly on disk. In the latter case you're assured of good references. If we do still detect that need within the community I believe we should keep that option open.

[mitchelsellers]

Leaving a DotNetNuke.Web.Mvc assembly without the extra dependencies leaves the same confusion that was the initial complaint, I don’t see a reason to differentiate between a “development” package than just adjusting the dependencies for the DotNetNuke.Web.Mvc package to include the extra items.

I do not support a bundle with every dll used by dnn for a few reasons.

1. Distribution of those assemblies might not be fully allowed if they aren’t on NuGet, we would need to validate the license terms and include relevant licenses if not.

2. The size of the package would be cumbersome in build environments such as azure DevOps and could introduce delays for individuals working with the platform and a perception of slowness or bloat (as this would be > 40-50 mb in size, more if deprecated included)

3. This could encourage direct dependencies on items that have DNN Api Wrappers to provide functions and could discourage best practice development on the platform by providing an easy way to bypass DNN’s API

[ohine]

The bundle method is how I've always used the references, I've also created a Dnn.Platform package which I fixed a few of the missing assemblies I needed for our development needs at nBrane. I usually only publish it manually when there is a breaking change in the API. Also, If we want to rename everything over to Dnn.Platform, I'm happy to transfer the ownership of that account.

As for the estimated 40-50mb size without the deprecated/telerik references, I don't see how that would be the case. At max, the DotNetNuke* assemblies without telerik is only around 5mb, if we're also including the personabar assemblies, it would be maybe 9mb. In addition, azure downloads things insanely quick, for example our devops package is downloading 1.2gb in source code and nuget packages in 43 seconds. We really need to make things easy and not require people to know every single interdependency when they're starting to develop in the platform.

I'm agree with @bdukes, we should keep the ability to reference a bundle, which depends on all of the packages. This way if someone just wants to reference the entire product, they can. Or if they want the granularity to select just a small hand full of packages they actually require, they can do that too.

[donker]

The drop off of package size at 9.1.0 is entirely due to the removal of Telerik from the package.

[ohine]

Interesting, any idea why v9.3.0 also shrunk?

[mitchelsellers]

@donker WHat is included in that package? Is that pre-post installation from the /bin directory?

I'm fine keeping the Bundle if desired, but would still want it to be a meta and only publicized assemblies that we have the rights to distribute and that people should be referencing, which is NOT all of the files in the /bin directory

[mitchelsellers]

Regarding size, an installed 9.2.1 installation of DNN with all references in 45.6Mb in the /bin directory with 32Mb of that being Telerik. Not horrible

[donker]

@mitchelsellers This is simply unpacking the install zip and adding all dlls in the bin folder to a package. So that excludes post installation dlls like those of included modules. I think this is fair as this is what is guaranteed to be in a default installation and nothing more.

@ohine The difference for 9.3.0 is the lack of the pdbs. I automatically find them when I have the symbols zip of the same name and included them. So for 9.3.0 it was a prerelease so no symbols package.

[donker]

@mitchelsellers Can you point me to a resource where I can see how one can differentiate between distributing inside a zip from Github and inside a package from NuGet? I'm having a hard time wrapping my head around this. I can imagine excluding any commercial product but by now we're on open source and framework dlls from what I can see.

[donker]

@mitchelsellers As to "3. This could encourage direct dependencies on items that have DNN Api Wrappers to provide functions and could discourage best practice development on the platform by providing an easy way to bypass DNN’s API":

We've had an on and off approach to "wrapping" 3rd party code for a long time now. Whether it's the wrappers around Telerik, or those around SharpZipLib. I understand the advantages:

1. Developers are not left wondering where to find stuff.
2. We can switch out the wrapped component at a later date and not break any module.

These are my arguments against this practice:

1. Every wrapper adds code which needs to be maintained.
2. Time and again we see that wrappers are fine for the Hello World case, but just don't cut it for advanced module development. Case in point: Compression. The actual compression methods I need are rarely in the wrapper.
3. It turns out that wrapping does little toward Sql Error Box #2 above. Case in point: Telerik. There is no way to squeeze for instance React components into the old wrapper. Technology moves forward and our wrappers are fast outdated. This means we need to maintain these legacy wrappers for a long time even after some technology has died. Longer than if we would have simply not wrapped them.

So from the point of view of the advanced module developer I'd like to have the option to use the dlls myself and deal with the fallout once those 3rd party dlls are updated or removed.

Following your logic this "KitchenSink" package would only ever be used by advanced module developers whereas we provide everything for basic module development in targeted packages like Development.Mvc. Hence my push for clear naming as well.

[mitchelsellers]

@donker The concern regarding distribution is around the fact that any DLL that we distribute that is not a dependency resolved by NuGet needs to have the proper license information included as part of the download so that the consumer is aware of the license that they are being bound by. We acheive this with a DNN Installation by including the respective license text within the /Licenses folder fo the DNN installation. (Which as has been discussed in the TAG group already has problems of its own that I'd rather not discuss in public.)

Regarding dependencies for other assemblies, I get your point, but we have fought for the past 9 months with backlash on breaking changes and dependencies that impacted module developers. We can distribute a package with a warning "Don't to this' but we have established that it doesn't do anything to actually stop people from doing it, and it doesn't stop the backlash on the community from breaking changes.

If your desire from point 3 above is to not include these functions as part of the API's and require developers to find their own solutions I think that would be providing a disservice to the consumers and would introduce a huge breaking change if we removed all of those wrappers and encouraged outside references. Additionally, by DNN providing wrappers, and supporting the wrapper interface we have more control over upgrades to the dependent packages. If we encourage others to reference outside of DNN, we lose some control over best recommendations here

Looking further into your thoughts of a Kitchen Sink sort of package, there is at least 1 assembly that if you did this would still not have all of the proper dependencies, and that is the DNN.AzureConnector.dll as it has Azure API dependencies that come in at installation. I'm not sure if there are others.

As I look at developer experience, I believe strongly that we should be publishing safe, supported, routes for development with the platform. I believe if we do things to support a "what if" or a special edge case that could be misconstrued we will simply be adding to the negative view of "its hard" by getting someone started with the platform and then having everything fall apart in the end. A prime example of this would have been the larger impact of the change to the ZIp library if more people had been referencing it directly than those where with the last upgrade to that package.

[donker]

@ahoefling pointed out in yesterday's meeting that the word "development" may be confused to mean "not finished". In that case "bundle" would be a fine alternative. So DotNetNuke.Bundle.Mvc used for MVC style modules.

[donker]

"If your desire from point 3 above is to not include these functions as part of the API's and require developers to find their own solutions I think that would be providing a disservice to the consumers and would introduce a huge breaking change if we removed all of those wrappers and encouraged outside references."

My point was to phase them out over time, not delete them. As we morph this platform I'm interested in pairing down the amount of code so we never get to the current situation any more. I just don't think we can afford to provide wrappers for everything any longer.

[mtrutledge]

Every time I use the Dotnetnuke packages from NuGet I am including the individual packages and always wondered why there isn't a bundle for API / MVC. I find that the current Bundle has more than I usually use for module development. I agree with @donker that we should have DotNetNuke.Bundle.MVC, DotNetNuke.Bundle.API, etc. packages. To aid in preventing users from using the old NuGet packages we can mark them as deprecated and add a note with a link to the new bundles. This way they are still available on NuGet, but NuGet will give a warning.

I am still on the fence if there should be a bundle with all the DLLs. I suppose it would not hurt and Microsoft does allow you to get all of NET Core from NuGet or just the pieces you want. The nuspec file for the All bundle should reference the other packages as dependencies and not include the various Dlls on its own.

[donker]

Upon debugging more NuGet issues I discovered one source of errors for me. The DotNetNuke.WebApi.nuspec includes a straightforward framework ref for system.web.http:

    <frameworkAssemblies>
      <frameworkAssembly assemblyName="System.Web.Http" targetFramework="net40" />
    </frameworkAssemblies>

This should be replaced with a specific ref as we roll out with this dll. So it should be:

    <dependencies>
      <dependency id="Microsoft.AspNet.WebApi.Core" version="5.2.3" />
      <dependency id="Microsoft.AspNet.WebApi.Client" version="5.2.3" />
      <dependency id="NewtonSoft.Json" version="7.0.1" />
      <dependency id="DotNetNuke.Web" version="$version$" />
    </dependencies>

This will add the correct version of System.Web.Http.dll to your project

[mitchelsellers]

@donker Thank you for this additional information.

Since the last update on this thread, a discussion with Microsoft has occured, about this, as well as other initiatives. Based on the information provided by them most of the above process was deemed effective, with an emphasis on ensuring that we use dependencies as noted by Peter Donker with specific versions, rather than framework directives.

To this point, I will be submitting a Pull Request based on that feedback, this discussion and a review of the NuGet package recommendations to remove warnings associated with the generation of the package. All above elements will be included.