Configure both Microsoft.AspNetCore.Http.Json.JsonOptions & Microsoft.AspNetCore.Mvc.JsonOptions using DocumentJsonSerializerOptions defaults #16836
Conversation
….AspNetCore.Mvc.JsonOptions using DocumentJsonSerializerOptions defaults
hishamco
changed the title
| => options.SerializerOptions.Merge(_jsonSerializerOptions); | ||
|
|
||
| public void Configure(MvcJsonOptions options) | ||
| => options.JsonSerializerOptions.Merge(_jsonSerializerOptions); |
There was a problem hiding this comment.
When does this get called for you? Because it doesn't for me.
Also, this would be better in two classes.
There was a problem hiding this comment.
When does this get called for you? Because it doesn't for me.
Read the issue description
Also, this would be better in two classes.
The problem with two classes is the naming
There was a problem hiding this comment.
We could use partial class for clarity
There was a problem hiding this comment.
I understand the issue description. My question is, when testing this, when do you see line 23 be called?
You can have two IConfigureOptions implementations, there's surely no difficulty in that.
|
I think we should not do this in OC because this will change the default of ASP.NET framework by OC which could impact people in a way they don't expect. I actually think we should remove I would do this instead #16831 (comment) |
|
This pull request has merge conflicts. Please resolve those before requesting a review. |
|
Fixed in #16837. |
This pull request includes changes to the
JsonOptionsConfigurationsclass to support bothHttpJsonOptionsandMvcJsonOptionsconfigurations.Fixes #16831
/cc @wAsnk