search

dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.63k stars 3.15k forks source link

Please use PrivateAssets when referencing Castle #33450

Closed mrpmorris closed 5 months ago

mrpmorris commented 5 months ago

Often I type ILogger in my code and Visual Studio automatically guesses what I am writing, unfortunately it nearly always assumes I want Castle.Core.Logging.ILogger - which I never do.

Would it be possible to mark references to the Castle libraries (and perhaps others?) as PrivateAssets so this stops happening?

EwoutdBoer commented 5 months ago

Probably they use it for the dynamic proxy. Would be nice to have a native implementation of this in the base library. Then Lots of referenties to castle could be removed

ajcvickers commented 5 months ago

This would be a breaking change with minimal value, so it's not something we plan to do.

mrpmorris commented 5 months ago

What's wrong with having a breaking change in a major release?

I imagine 99.999% of EFCore users don't use Castle in their apps too. It might slightly inconvenience a tiny percentage of people, but this catches me out multiple times per day so inconveniences me repeatedly.

Other than those few people having to add a nuget reference to Castle, would it have any other negative effects that I'm not thinking of?

mrpmorris commented 5 months ago

Here is a video comparing my productivity before installing EFCore Proxies and afterwards. Considering I inject ILogger into just about every class I write, you can see how much this is slowing me down - especially when it's not until compile time that I notice VS has chosen Castle.

https://github.com/dotnet/efcore/assets/3111981/099987ff-5858-43d9-96d6-998f52edb1f1

EwoutdBoer commented 5 months ago

From my perspective: Making the dependency private is not solving the 'root' cause. I rather not have a dependency on Castle or any other externally managed package(s). Just the Xz issue as an example, or for example changing licenses like Identity framework. Therefore I was glad Microsoft created an alternative for Newtonsoft Json. When having the Castle sources creating a .net framework internal dynamic proxy not that hard.

roji commented 5 months ago

@EwoutdBoer expecting Microsoft to implement everything to replace well-working, open-source components isn't reasonable, nor is it a good way to have an open-source ecosystem - I'm still not sure why you're suggesting we do this or what it would bring to the community.

mrpmorris commented 5 months ago

@roji I agree. Castle is well-established, reliable, and generally excellent overall. I don't want MS to provide everything in the World either.

Apps that consume the Proxies package would still work if the Castle dependency was PrivateAssets, wouldn't they? Am I correct in thinking only the few people who also use that Castle package in their app would just have to add a nuget package, and those who don't use it wouldn't be affected?

fiseni commented 5 months ago

If it's added as PrivateAsset, then the Castel library won't be included in your build output (of your app). If the Proxies package needs the Castle on runtime, then it won't be able to find the dll. Isn't that correct?

mrpmorris commented 5 months ago

@fiseni I don't know how it works. Obviously if it breaks it then it can't be done.

It would be nice if there was a way to use something and have it compiled along with the consuming app, but be a shadow reference so the consumer doesn't see the classes directly unless they add the nuget directly.

This is a real pain for me.

ErikEJ commented 5 months ago

Keep in mind that only a fraction of EF Core users use lazy loading...

mrpmorris commented 5 months ago

@ErikEJ I advise people install it but don't use it.

I have a hook that detects lazy-load and in development throws an exception for the developer to fix it, but in production allows the slower performance in order to get the correct state (and log a warning).

This gives the best of both worlds.

But still, there are a thousand times more Lazy-Load users than there are people who use Castle to generate runtime classes.

EwoutdBoer commented 5 months ago

@roji Totally agree that Microsoft should not implement all open source 'functionalities'. I like the Castle project very much and used it heavily in the dotnet framework period. Their Wcf facility is still fantastic and made working with Wcf so much better, simpler and more efficient. From my perspective something like the castles Dynamic proxy is a base functionality of an IoC implementation, like the named instances which was added recently.