search

dotnet / razor

Compiler and tooling experience for Razor ASP.NET Core apps in Visual Studio, Visual Studio for Mac, and VS Code.
https://asp.net
MIT License
494 stars 191 forks source link

Don't use service locator pattern where it's not required #6955

Open ryanbrandenburg opened 1 year ago

ryanbrandenburg commented 1 year ago

Roslyn makes extensive use of the "service locator pattern" for their DI systems. That seems to work well for them (and has reportedly had significant performance benefits), but it is not recommended in general.

When I moved Razor to CLaSP fresh off my work in the Roslyn repo I proliferated this pattern in many placed I touched. Let's remove those usages which aren't required (say for a self-destructing service or one which is not available when the class is constructed). They should all be off of either requestContext.GetRequiredService or requestContext.LspServices. Then we can consider adding a deprecated attribute to these properties to avoid their over-use.

davidwengier commented 1 year ago

💯

AFAIK roslyn only does this for scoped services (ie, services specific to a workspace instance, or language for the C#/VB split). For us, where everything (I think) is singleton, I much prefer being declarative about it, and using constructors.

davidwengier commented 3 months ago

Moving to backlog. I audited the usages of this, and whilst it's still good to do, the debt is all in areas that will be removed eventually with cohosting anyway.