Proposal: Controller.Json() overloads are confusing in 3.0, expose extensions methods.

[NickCraver]

In previous versions of ASP.NET Core, the overloads for rendering Json were:

public virtual JsonResult Json(object data, JsonSerializerSettings serializerSettings);
public virtual JsonResult Json(object data);

Here, JsonSerializerSettings is Newtonsoft.Json.JsonSerializerSettings.

Since decoupling the dependency in 3.0 though, it's now:

public virtual JsonResult Json(object data, object serializerSettings);
public virtual JsonResult Json(object data);

While this makes sense to decouple from Newtonsoft specifically, it makes for a poor user experience. For example, I couldn't readily find what type I'm supposed to be using for the serializerSettings argument.

Is there harm in adding additional overloads that are specific? For example, the Microsoft.AspNetCore.Mvc.NewtonsoftJson package could add a:

public static Json(this Controller controller, object data, JsonSerializerSettings serializerSettings);

It'd expose a specific overload for the user to use. The same for System.Text.Json.

If all of this isn't doable for API compat reasons, at the very least the docs should be updated. The out-of-the-box experience with System.Text.Json is very confusing and leaves a user lost on what they're supposed to be doing.

[rynowak]

Unfortunately extension methods don't really help with this. We already considered this during the initial design and rejected it for the following reason.

When you use an extension method on this you have to qualify it. So it means that in the following example you won't see the extension method offered as an example.

public class MyController : Controller
{
    public IActionResult Foo()
    {
        return Json(...); // Can't resolve Json(this Controller controller, object data, JsonSerializerSettings serializerSettings);
    }
}

You'd have to write:

public class MyController : Controller
{
    public IActionResult Foo()
    {
        return this.Json(...); // Can resolve Json(this Controller controller, object data, JsonSerializerSettings serializerSettings);
    }
}

[NickCraver]

Ah that's a great point. I guess it's a docs issue then, though the API is unfriendly/unclear and that's unfortunate. My current workaround is creating an overload on my base controller class:

protected JsonResult Json(object data, System.Text.Json.Serialization.JsonSerializerOptions serializerOptions = null)
    => serializerOptions != null
    ? base.Json(data, serializerOptions)
    : base.Json(data);

...just in case that helps anyone else. Note calling Json(data, (anything)null) throws an error in the default Json(object, object) overload, so you have to make sure not to call it with one.

[rynowak]

@pranavkm - that last part sounds like something we should fix.

[NickCraver]

Null overload issue was fixed in #12207 - thanks!

[NickCraver]

@rynowak Since we can't fix things in this way and the documentation has improved it seems I should just close this out, any objections?

[rynowak]

Yep, this can be closed.