Skip to content
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

Don't forget about JsonSerializerOptions.CreateForWeb() #34626

Closed
jozkee opened this issue Apr 7, 2020 · 17 comments · Fixed by #36073
Closed

Don't forget about JsonSerializerOptions.CreateForWeb() #34626

jozkee opened this issue Apr 7, 2020 · 17 comments · Fixed by #36073
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json good first issue Issue should be easy to implement, good for first-time contributors
Milestone

Comments

@jozkee
Copy link
Member

jozkee commented Apr 7, 2020

This is a leftover from #32937, we didn't add the default options that are meant to sync the options between System.Net.Http.Json and ASP.NET.

As per the spec, the next API should be added as well.

namespace System.Text.Json
{
    public partial class JsonSerializerOptions
    {
        public static JsonSerializerOptions CreateForWeb();
    }
}

Above method should return an options instance as the following:

new JsonSerializerOptions 
{ 
    PropertyNameCaseInsensitive = true, 
    PropertyNamingPolicy = JsonNamingPolicy.CamelCase 
}
@jozkee jozkee added this to the 5.0 milestone Apr 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Apr 7, 2020
@jozkee
Copy link
Member Author

jozkee commented Apr 7, 2020

cc @terrajobst @layomia

@jozkee jozkee added api-ready-for-review and removed untriaged New issue has not been triaged by the area owner labels Apr 7, 2020
@ericstj
Copy link
Member

ericstj commented Apr 8, 2020

This is one we only plan to do for 5.0, correct? (not in the blazor release)

@jozkee
Copy link
Member Author

jozkee commented Apr 8, 2020

As per our early discussions, yes, this only applies for .NET 5, on the code for 3.1 we define the options directly in the Http.Json project as is done in ASP.NET.

@terrajobst terrajobst added blocking Marks issues that we want to fast track in order to unblock other important work api-approved API was approved in API review, it can be implemented and removed api-ready-for-review blocking Marks issues that we want to fast track in order to unblock other important work labels Apr 14, 2020
@terrajobst
Copy link
Member

terrajobst commented Apr 16, 2020

Video

  • Since we already have a constructor, we'd prefer an overload instead of a factory because it makes it more discoverable.
namespace System.Text.Json
{
    public enum JsonSerializerDefaults
    {
        General,
        Web
    }
    public partial class JsonSerializerOptions
    {
        public JsonSerializerOptions(JsonSerializerDefaults defaults);
    }
}

@layomia layomia added help wanted [up-for-grabs] Good issue for external contributors good first issue Issue should be easy to implement, good for first-time contributors labels Apr 17, 2020
@jaymin93
Copy link
Contributor

jaymin93 commented Apr 21, 2020

@layomia @terrajobst I want to work on this , I am new contributor hence may be I need your help to getting started and contribute first time in .net .

what i understood is i have to create overloaded ctor with enum argument .

is it ok and should i go ahead ?

@layomia
Copy link
Contributor

layomia commented Apr 21, 2020

@jaymin93 thanks for your interest!

Please see https://github.com/dotnet/runtime/tree/master/docs/workflow/building/libraries for steps on how to build the repo and run the tests.

what i understood is i have to create overloaded ctor with enum argument .

Yes, this is right. The implementation for this will add new public API. Please see this recent JSON PR that adds new API as an example - #34725.

Please let me know if you run into any issues.

@layomia layomia removed the help wanted [up-for-grabs] Good issue for external contributors label Apr 21, 2020
@jaymin93
Copy link
Contributor

jaymin93 commented Apr 21, 2020

@layomia from above your mentioned pull request i can see already implemented parameterized constructor so this need to add as paste here in this issue then ?

i am quite new here so please guide me if i am wrong at any place

@layomia
Copy link
Contributor

layomia commented Apr 21, 2020

@jaymin93, not exactly. You need to add a new constructor & JsonSerializerDefaults enum as described in #34626 (comment).

@jaymin93
Copy link
Contributor

That part I understood but after adding ctor and parameter, what I have to do with parameter of constructor? I have to call base constructor as well from my new constructor???

@layomia
Copy link
Contributor

layomia commented Apr 22, 2020

@jaymin93 - I believe the behavior here would be -

public JsonSerializerOptions(JsonSerializerDefaults defaults)
{
    if (defaults == JsonSerializerDefaults.Web)
    {
        _propertyNameCaseInsensitive = true, 
        _propertyNamingPolicy = JsonNamingPolicy.CamelCase 
    }
}

cc @jozkee PTAL

@jaymin93
Copy link
Contributor

ok got it let me go ahead with this , thanks for your help :)

@jozkee
Copy link
Member Author

jozkee commented Apr 22, 2020

@rynowak @pranavkm does this satisfy your (ASP.NET) scenario?
I saw here https://github.com/dotnet/aspnetcore/pull/21016/files#r411571846 that you also want to have Encoder set to JavaScriptEncoder.UnsafeRelaxedJsonEscaping as default.

@jaymin93
Copy link
Contributor

@layomia after executing build steps for libs of Json , i am not able to open in vs and cmd shows this error on my 2 computer , although i can see that it has started to install preview of 5.0.100-preview.4.20202.8 and without any error it builds but later executing
pushd ../src & dotnet build & popd & dotnet build /t:test

it shows that it does not have preview sdk with below error message

A compatible installed .NET Core SDK for global.json version [5.0.100-preview.4.20202.8] from [G:\sdk\New\runtime\global.json] was not found
Install the [5.0.100-preview.4.20202.8] .NET Core SDK or update [G:\sdk\New\runtime\global.json] with an installed .NET Core SDK:
1.1.6 [C:\Program Files\dotnet\sdk]
1.1.7 [C:\Program Files\dotnet\sdk]
1.1.8 [C:\Program Files\dotnet\sdk]
1.1.9 [C:\Program Files\dotnet\sdk]
1.1.10 [C:\Program Files\dotnet\sdk]
1.1.11 [C:\Program Files\dotnet\sdk]
1.1.12 [C:\Program Files\dotnet\sdk]
1.1.13 [C:\Program Files\dotnet\sdk]
1.1.14 [C:\Program Files\dotnet\sdk]
2.1.2 [C:\Program Files\dotnet\sdk]
2.1.4 [C:\Program Files\dotnet\sdk]
2.1.101 [C:\Program Files\dotnet\sdk]
2.1.103 [C:\Program Files\dotnet\sdk]
2.1.104 [C:\Program Files\dotnet\sdk]
2.1.200 [C:\Program Files\dotnet\sdk]
2.1.201 [C:\Program Files\dotnet\sdk]
2.1.202 [C:\Program Files\dotnet\sdk]
2.1.300 [C:\Program Files\dotnet\sdk]
2.1.400 [C:\Program Files\dotnet\sdk]
2.1.401 [C:\Program Files\dotnet\sdk]
2.1.402 [C:\Program Files\dotnet\sdk]
2.1.403 [C:\Program Files\dotnet\sdk]
2.1.500 [C:\Program Files\dotnet\sdk]
2.1.502 [C:\Program Files\dotnet\sdk]
2.1.503 [C:\Program Files\dotnet\sdk]
2.1.504 [C:\Program Files\dotnet\sdk]
2.1.505 [C:\Program Files\dotnet\sdk]
2.1.507 [C:\Program Files\dotnet\sdk]
2.1.508 [C:\Program Files\dotnet\sdk]
2.1.509 [C:\Program Files\dotnet\sdk]
2.1.602 [C:\Program Files\dotnet\sdk]
2.1.700 [C:\Program Files\dotnet\sdk]
2.1.701 [C:\Program Files\dotnet\sdk]
2.1.801 [C:\Program Files\dotnet\sdk]
2.1.802 [C:\Program Files\dotnet\sdk]
2.2.104 [C:\Program Files\dotnet\sdk]
2.2.202 [C:\Program Files\dotnet\sdk]
2.2.300 [C:\Program Files\dotnet\sdk]
2.2.301 [C:\Program Files\dotnet\sdk]
2.2.401 [C:\Program Files\dotnet\sdk]
2.2.402 [C:\Program Files\dotnet\sdk]
3.1.201 [C:\Program Files\dotnet\sdk]

even i am facing issues currently i want to complete this task .if you can help me what i can do here ?
Thanks

@ericstj
Copy link
Member

ericstj commented Apr 22, 2020

You can use the dotnet that is downloaded in the .dotnet folder. Eg: G:\sdk\New\runtime\.dotnet\dotnet.exe

@jaymin93
Copy link
Contributor

jaymin93 commented Apr 23, 2020

Yes I can see it has downloaded and exist on path you mentioned but visual studio shows error when I open solution and further if I change version in global json then it shows error in code so what should I do here ?

one more question do you mean this downloaded preview sdk i can use in visual studio without error ? if so please suggest how to do that ?

@jaymin93
Copy link
Contributor

You can use the dotnet that is downloaded in the .dotnet folder. Eg: G:\sdk\New\runtime\.dotnet\dotnet.exe

can you please guide further ?

@layomia @terrajobst can you also help here please ?

Yes I can see it has downloaded and exist on path you mentioned but visual studio shows error when I open solution and further if I change version in global json then it shows error in code so what should I do here ?

one more question do you mean this downloaded preview sdk i can use in visual studio without error ? if so please suggest how to do that ?

@ericstj
Copy link
Member

ericstj commented Apr 27, 2020

If you're just building from the commandline, use the dotnet.exe that I suggested. That should work using the steps you previously tried here: #34626 (comment)

Check out the contributing doc for more information: https://github.com/dotnet/runtime/blob/master/docs/workflow/README.md
https://github.com/dotnet/runtime/blob/master/docs/workflow/requirements/windows-requirements.md

Specifically:

The dotnet/runtime repository requires at least Visual Studio 2019 16.6 Preview 2.

You can also use build -vs System.Text.Json from the commandline, see https://github.com/dotnet/runtime/blob/8e0147ecdfd2717362ca0a859089968bad17aefc/docs/workflow/testing/libraries/testing.md#running-tests-from-visual-studio

/cc @ViktorHofer

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json good first issue Issue should be easy to implement, good for first-time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants