Closed ZodmanPerth closed 5 months ago
ghost
commented
5 months ago Hello ZodmanPerth, thank you for opening an issue with us!
I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌
michael-hawker
commented
5 months ago The MVVM Toolkit doesn't have any extra dependencies outside the core .NET runtime for .NET 6+.
There's very little in terms of content and classes for each of the areas you've mentioned, there should be minimal overhead to including them and not using them, especially with trimming. I don't think it's worth the effort and confusion of splitting these things apart for the standard usage.
This is probably more of a NuGet/.NET dependency question about hiding internal dependencies when referencing other packages. I know there's some issue I think open for this about not bleeding APIs to parent packages, if not desired unless the consumer explicitly references them. I think that's the scenario that's more applicable here: https://github.com/NuGet/Home/issues/10375
@Sergio0694 what are your thoughts?
Sergio0694
commented
5 months ago "representing overheads and a larger footprint"
What overhead? What larger footprint?
ZodmanPerth
commented
5 months ago @Sergio0694
What overhead?
The review process (operational overhead). When one of our developers uses part of the MVVM Toolkit we don't want to use to build a solution to a problem, creates a PR, is reviewed by a colleague who finds the problem and sends it back. That's an overhead that should be avoided as it wastes a lot of time. Better the developer does not have access to the parts of the API we don't want them to us.
What larger footprint?
The parts of the API we don't want to use. If every NuGet dependency we take did this, a mediam-large scale application would be twice the size it needs to be. When you're targeting low-end devices this becomes an issue.
ZodmanPerth
commented
5 months ago @michael-hawker
There's very little in terms of content and classes for each of the areas you've mentioned...
I concede that's true today, though I still prefer to be able to remove code we don't use as it adds up to a larger footprint. There's also the fact that libraries such as the CommunityToolkit tend to grow over time, adding more functionality that we won't require, which is excess bloat to our solution.
This is probably more of a NuGet/.NET dependency question about hiding internal dependencies when referencing other packages.
I agree, this is our main issue. NuGet/Home#10375 does address this need, though it doesn't appear to hold much traction.
In my experience the best way to resolve this situation with the current functionality of NuGet is to break the NuGet package into smaller packages as I described in the original post. I believe library providers should be striving for the smallest possible packages, allowing consumers to pick and choose. If you want to wrap up several packages into a larger package like the MVVM package today you could create a master package that buncles them together.
It's not pretty I agree, but given the current capabilities of NuGet packaging without NuGet/Home#10375 or similar, I can't see any other way for library creators to package features in a way that make sense for all the consumers.
I applaud and appreciate the Community Toolkit for breaking things down as they did in version 8. This is the approach I believe the MVVM toolkit should take. In my opinion, people who build libraries for other consumers should be aiming for multiple small packages instead of one large one.
Sergio0694
commented
5 months ago "The review process (operational overhead). When one of our developers uses part of the MVVM Toolkit we don't want to use to build a solution to a problem, creates a PR, is reviewed by a colleague who finds the problem and sends it back. That's an overhead that should be avoided as it wastes a lot of time. Better the developer does not have access to the parts of the API we don't want them to us."
This seems like a complete non issue to me. There's quite literally thousands and thousands of APIs in the BCL alone, that developers would be able to use incorrectly. Referencing less NuGet packages is not a way to save time on PR reviews.
You could also check out the banned APIs analyzer.
"The parts of the API we don't want to use. If every NuGet dependency we take did this, a mediam-large scale application would be twice the size it needs to be. When you're targeting low-end devices this becomes an issue."
That is completely not true. The MVVM Toolkit is fully trimmable. It could literally be 20 MB or more worth of IL code (which is what many libraries out there are like, such as the Windows SDK projections), and it would just shrink down to just the code you actually need at runtime. All the code that's not used will just go away. If you're not using trimming, I would strongly recommend doing so. That is the recommended way to reduce the binary size in modern .NET applications.
If you don't plan on enabling trimming, adding too many libraries can ultimately increase your package size. But at the same time, binary size is not a concern if you are not using trimming. Either way it's not on the MVVM Toolkit (or any other package) to break the package down into smaller packages for the sole reason to provide negligible size improvements to people that are not using trimming, it's unfortunately just not scalable and not something that we use as a guiding principle at all.
"I applaud and appreciate the Community Toolkit for breaking things down as they did in version 8."
That was a completely different issue. The problem is that those dependencies were not trimmable, which meant they were causing considerable package size increase even in cases where functionality wasn't used. That is not the case at all here.
I'm going to close this, since it's not actionable and there isn't anything we plan on doing in this direction 🙂 Thank you!
ZodmanPerth
commented
5 months ago There's quite literally thousands and thousands of APIs in the BCL alone, that developers would be able to use incorrectly.
That's a different context. I would expect a framework to have a broad functional scope with functionality we won't use. Frameworks are foundations of apps - solutions are build on top of frameworks, so its expected apps will have unused functionality provided by the framework. However a 3rd party Library is additive functionality. Ideally libraries have functionality focussed on specific topics that enable them to be added without creating a large change in footprint due to the functional scope being too broad. In the case of existing large codebases adopting the MVVM toolkit I am suggesting a single MVVM package is too broad, and have suggested a way to break it down which makes sense in the scenario I'm trying to adopt it.
However...
The MVVM Toolkit is fully trimmable.
I did not know such a thing existed. After a bit of research I found a blog post on the topic (written by you as it turns out) which will serve as a great starting point for us to try this out: Leveraging trimming to make the Microsoft Store Faster and Reduce its Binary Size.
You could also check out the banned APIs analyzer.
This appears to be what we need to prevent accidentally using parts of the library we don't want. Thanks for pointing this out.
Agree to close this issue. Thanks again.
As of v8.2.2 the MVVM Nuget package includes several different MVVM concepts:
Bundling these together into a single package has benefits for small/fresh solutions to provide a quick jump start. However for large/mature projects that are already implementing these concepts in different ways, the MVVM package (as light as v8 is) still provides implementations that will already be implemented by other means, representing overheads and a larger footprint. For example, my project is looking to adopt ObservableObjects and possibly Commanding but is already heavily invested in a different Dependency Injection library and proactively rejects a Messaging architectural pattern.
Instead of (or as well as) a single MVVM package, it would be beneficial to provide separate packages around clear conceptual boundaries. Then a project such as mine could pick and choose which concepts to include (e.g. just ObservableObjects and Commanding) without the Dependency Injection and Messaging parts.
Example breakdown:
This would allow options for gradually introducing CommunityToolkit.MVVM into large/mature projects without having such a heavy footprint and would de-risk adoption of architectural patterns that are not desired.