-
-
Notifications
You must be signed in to change notification settings - Fork 916
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
Update middleware sample #2903
Update middleware sample #2903
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2903 +/- ##
=======================================
Coverage 83.77% 83.77%
=======================================
Files 366 366
Lines 15017 15017
Branches 2375 2375
=======================================
Hits 12580 12580
Misses 1802 1802
Partials 635 635 Continue to review full report at Codecov.
|
@@ -56,6 +56,7 @@ public void ConfigureServices(IServiceCollection services) | |||
// add infrastructure stuff | |||
services.AddHttpContextAccessor(); | |||
services.AddLogging(builder => builder.AddConsole()); | |||
services.AddSingleton<GraphQLMiddleware>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did it work before? I mean app.UseMiddleware<GraphQLMiddleware>();
without services.Add
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All services from ctor and Invoke
are discovered and pulled by ASP.NET subsystem, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, yes that was correct, as it was "ASP.Net Core convention-based middleware". It was essentially a scoped/transient service, and DI services could be added to either the constructor or Invoke method.
With this change, the middleware is pulled from DI and it is called "ASP.Net Core factory-based middleware".
I think it's just a clearer sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about middleware from server repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll review when I look at the serialization
ISchema
via DIIMiddleware
interface