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

Adds more features back that were lost due to the removal of Newtonsoft #16292

Merged
merged 21 commits into from
Jun 26, 2024

Conversation

gvkries
Copy link
Contributor

@gvkries gvkries commented Jun 11, 2024

Added ToObject<T> implementations back to JsonDynamicObject, JsonDynamicArray and JsonDynamicValue.
This functionality allows to convert JSON to the strongly typed content items and parts when using dynamic typing. E.g. we have this functionality referenced in the documentation:

var anchors = (Anchor[])Model.ContentItem.Content.Blog.Image.Anchors.ToObject<Anchor[]>();

Also something like the following is now possible again:

@foreach (var blogPost in Model.ContentItem.Content.NamedBagPart.ContentItems)
{
    var contentItem = blogPost.ToObject<ContentItem>();
    
    [...]
}

To support value comparisons when using dynamic typing, I made the following changes:

  1. Implemented equality operators in JsonDynamicValue.
  2. Added implicit conversions from base types to JsonDynamicValue.
  3. Implemented IComparable in JsonDynamicValue.

E.g. this allows using conditions with dynamic values, without casting to the target type before:

dynamic blogPost = ...;
if (blogPost.BlogPost.Subtitle.Text == "Test") {... }

Note that the comparison implementation (<, >, <=, >=) is very basic at the moment, e.g. it does not handle all types
a JsonDynamicValue can be converted too. The comparison was therefore supporting a wider range
of target types with Newtonsoft JSON, in contrast to this rudimentary implementation.

Fixes #16233
Fixes #16221
Fixes #16290

/cc @giannik @MikeAlhayek @sebastienros

Copy link
Member

@MikeAlhayek MikeAlhayek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. If you are able to, it may not be a bad idea to add functional tests for this to ensure everything is working and will continue to work as expected.

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 11, 2024

The unit test I mentioned in issue #16290 doesn't work with this PR , any plans to fix it in this PR?

Can we modify the DynamicJsonValue fetching behaviour so that it can be generated from the JsonValueKind enumeration, e.g.

JsonValueKind.String map to System.
JsonValueKind.Number map to Decimal.
JsonValueKind.Object map to DynamicJsonValue.
...others

I'm working on that.

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 11, 2024

Similar functionality exists in Jint and ClearScript, Jint uses ExpandoObject, but ClearScript uses DynamicObject.
Shall we try ExpandoObject as well? I've tried using ClearScript before because of the same type conversion or value compatibility issues.

microsoft/ClearScript#485

sebastienros and others added 4 commits June 11, 2024 10:59
…micValue.cs

Co-authored-by: Mike Alhayek <mike@crestapps.com>
…micValue.cs

Co-authored-by: Mike Alhayek <mike@crestapps.com>
…micValue.cs

Co-authored-by: Mike Alhayek <mike@crestapps.com>
@sebastienros
Copy link
Member

Dude you are so cool, but even more if you create a unit test with these conversions ;)

Comment on lines +238 to +241
public static bool operator !=(JsonDynamicValue left, JsonDynamicValue right)
{
return !(left == right);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static bool operator !=(JsonDynamicValue left, JsonDynamicValue right)
{
return !(left == right);
}
public static bool operator !=(JsonDynamicValue left, JsonDynamicValue right) => !(left == right);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you prefer expression bodies, but Sebastien removed all of them the last time this file was changed. So I rather keep the format consistent for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm. When I tried to open the classes and read what they were doing it was so condensed I couldn't understand what I was reading.

What I realized is that if the definition + body fits on a line on the screen then it's fine, if the implementation has to go on the second line then there is no point and it's more readable to just use a standard body.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But please don't change anything because "that one char can be removed and it will fit in a single line", there is no readability advantage IMO by squashing everything in a line.

Comment on lines +243 to +246
public static bool operator <(JsonDynamicValue left, JsonDynamicValue right)
{
return ReferenceEquals(left, null) ? !ReferenceEquals(right, null) : left.CompareTo(right) < 0;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static bool operator <(JsonDynamicValue left, JsonDynamicValue right)
{
return ReferenceEquals(left, null) ? !ReferenceEquals(right, null) : left.CompareTo(right) < 0;
}
public static bool operator <(JsonDynamicValue left, JsonDynamicValue right)
=> ReferenceEquals(left, null) ? !ReferenceEquals(right, null) : left.CompareTo(right) < 0;

Comment on lines +248 to +261
public static bool operator <=(JsonDynamicValue left, JsonDynamicValue right)
{
return ReferenceEquals(left, null) || left.CompareTo(right) <= 0;
}

public static bool operator >(JsonDynamicValue left, JsonDynamicValue right)
{
return !ReferenceEquals(left, null) && left.CompareTo(right) > 0;
}

public static bool operator >=(JsonDynamicValue left, JsonDynamicValue right)
{
return ReferenceEquals(left, null) ? ReferenceEquals(right, null) : left.CompareTo(right) >= 0;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static bool operator <=(JsonDynamicValue left, JsonDynamicValue right)
{
return ReferenceEquals(left, null) || left.CompareTo(right) <= 0;
}
public static bool operator >(JsonDynamicValue left, JsonDynamicValue right)
{
return !ReferenceEquals(left, null) && left.CompareTo(right) > 0;
}
public static bool operator >=(JsonDynamicValue left, JsonDynamicValue right)
{
return ReferenceEquals(left, null) ? ReferenceEquals(right, null) : left.CompareTo(right) >= 0;
}
public static bool operator <=(JsonDynamicValue left, JsonDynamicValue right)
=> ReferenceEquals(left, null) || left.CompareTo(right) <= 0;
public static bool operator >(JsonDynamicValue left, JsonDynamicValue right)
=> !ReferenceEquals(left, null) && left.CompareTo(right) > 0;
public static bool operator >=(JsonDynamicValue left, JsonDynamicValue right)
=> ReferenceEquals(left, null) ? ReferenceEquals(right, null) : left.CompareTo(right) >= 0;

Comment on lines 478 to 661

public static implicit operator JsonDynamicValue(short? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(int value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(int? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(long value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(long? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(sbyte value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(sbyte? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(float value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(float? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(string value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(ushort value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(ushort? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(uint value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(uint? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(ulong value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(ulong? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}

public static implicit operator JsonDynamicValue(byte[] value)
{
return new JsonDynamicValue(JsonValue.Create(Convert.ToBase64String(value), JOptions.Node));
}

public static implicit operator JsonDynamicValue(TimeSpan value)
{
return new JsonDynamicValue(JsonValue.Create(value.ToString(), JOptions.Node));
}

public static implicit operator JsonDynamicValue(TimeSpan? value)
{
return new JsonDynamicValue(value.HasValue ? JsonValue.Create(value.ToString(), JOptions.Node) : null);
}

public static implicit operator JsonDynamicValue(Uri? value)
{
return new JsonDynamicValue(value != null ? JsonValue.Create(value.ToString(), JOptions.Node) : null);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static implicit operator JsonDynamicValue(bool value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(bool? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(byte value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(byte? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(char value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(char? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(DateTime value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(DateTime? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(DateTimeOffset value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(DateTimeOffset? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(decimal value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(decimal? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(double value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(double? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(Guid value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(Guid? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(short value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(short? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(int value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(int? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(long value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(long? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(sbyte value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(sbyte? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(float value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(float? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(string value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(ushort value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(ushort? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(uint value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(uint? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(ulong value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(ulong? value)
{
return new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
}
public static implicit operator JsonDynamicValue(byte[] value)
{
return new JsonDynamicValue(JsonValue.Create(Convert.ToBase64String(value), JOptions.Node));
}
public static implicit operator JsonDynamicValue(TimeSpan value)
{
return new JsonDynamicValue(JsonValue.Create(value.ToString(), JOptions.Node));
}
public static implicit operator JsonDynamicValue(TimeSpan? value)
{
return new JsonDynamicValue(value.HasValue ? JsonValue.Create(value.ToString(), JOptions.Node) : null);
}
public static implicit operator JsonDynamicValue(Uri? value)
{
return new JsonDynamicValue(value != null ? JsonValue.Create(value.ToString(), JOptions.Node) : null);
}
public static implicit operator JsonDynamicValue(bool value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(bool? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(byte value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(byte? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(char value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(char? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(DateTime value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(DateTime? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(DateTimeOffset value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(DateTimeOffset? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(decimal value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(decimal? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(double value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(double? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(Guid value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(Guid? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(short value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(short? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(int value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(int? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(long value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(long? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(sbyte value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(sbyte? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(float value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(float? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(string value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(ushort value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(ushort? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(uint value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(uint? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(ulong value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(ulong? value)
=> new JsonDynamicValue(JsonValue.Create(value, JOptions.Node));
public static implicit operator JsonDynamicValue(byte[] value)
=> new JsonDynamicValue(JsonValue.Create(Convert.ToBase64String(value), JOptions.Node));
public static implicit operator JsonDynamicValue(TimeSpan value)
=> new JsonDynamicValue(JsonValue.Create(value.ToString(), JOptions.Node));
public static implicit operator JsonDynamicValue(TimeSpan? value)
=> new JsonDynamicValue(value.HasValue ? JsonValue.Create(value.ToString(), JOptions.Node) : null);
public static implicit operator JsonDynamicValue(Uri? value)
=> new JsonDynamicValue(value != null ? JsonValue.Create(value.ToString(), JOptions.Node) : null);

@gvkries
Copy link
Contributor Author

gvkries commented Jun 12, 2024

I added some basic unit tests for the new implicit conversions and operators.

Also, to make it even more complex, I added a fix for #16290 into this PR. Serializing the dynamic wrappers (JsonDynamicObject, JsonDynamicArray and JsonDynamicValue) should not include these objects in the output at all. They should be transparent for users, the same way as it was with NSJ, where we didn't had this extra abstraction layer.

…micValue.cs

Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
@hyzx86
Copy link
Contributor

hyzx86 commented Jun 12, 2024

I've tested it and found that it's still a little different from the old version, but it's almost complete. Thanks!

const content = contentItem('46t41hp7d9w6446r35405swzwt')

return {
    data: {
        // contentStr:JSON.stringify(content), //Error: Cyclic reference detected
        testValue: content.Content.SEAQuotaionForm.SoldTo.Text,
        testValue2: content.Content.SEAQuotaionForm.SoldTo.Text + "2123",
        QuoteId: content.Content.SEAQuotaionForm.QuoteId.Value + 10000,
        obj: JSON.parse(JSON.stringify(content.Content)),
        objStr: JSON.stringify(content.Content),
        objAll: content
    }
}

this PR
image

this is oc 1.8
image

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 12, 2024

I should try the datetime value again later.

@gvkries
Copy link
Contributor Author

gvkries commented Jun 12, 2024

I think in Jint you have the same problem. The dynamic wrappers are now also serialized, that's why you get a different result. The JsonDynamicObject only has the Count property public, and that's getting serialized.

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 12, 2024

I think in Jint you have the same problem. The dynamic wrappers are now also serialized, that's why you get a different result. The JsonDynamicObject only has the Count property public, and that's getting serialized.

Yes, in my tests the serialisation converter you defined works in Jint

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 12, 2024

I created a PR for solving serialisation problems in Jint #16298

@gvkries
Copy link
Contributor Author

gvkries commented Jun 13, 2024

I simplified this a little bit by adding a base class for the dynamic wrappers (as @hishamco suggested). Also, I removed the nested JsonDynamicMetaObject class as it is not required.

@gvkries gvkries requested review from hyzx86 and sebastienros June 13, 2024 11:40
@sebastienros
Copy link
Member

@MikeAlhayek @hishamco waiting for you asking for more things, or we'll merge in 24h.

Copy link
Member

@MikeAlhayek MikeAlhayek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

test/OrchardCore.Tests/Data/JsonDynamicTests.cs Outdated Show resolved Hide resolved
@hishamco
Copy link
Member

@sebastienros if there's any objection regarding expression bodies please let me, because it was a useful feature for single-line methods

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 18, 2024

@sebastienros if there's any objection regarding expression bodies please let me, because it was a useful feature for single-line methods

Personally, it's not a big deal, and I'm more than inclined to prioritise readability, too
The more urgent thing right now should be to resolve the remaining almost 1000 issues 😅
If nothing else is added, I suggest merging it as soon as possible

Comment on lines +40 to +43
public void Merge(JsonNode? content, JsonMergeSettings? settings = null)
{
_jsonObject.Merge(content, settings);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public void Merge(JsonNode? content, JsonMergeSettings? settings = null)
{
_jsonObject.Merge(content, settings);
}
public void Merge(JsonNode? content, JsonMergeSettings? settings = null) => _jsonObject.Merge(content, settings);

Comment on lines +9 to +12
public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotSupportedException($"Deserializing a {typeof(T).Name} is not supported.");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
throw new NotSupportedException($"Deserializing a {typeof(T).Name} is not supported.");
}
public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
=> throw new NotSupportedException($"Deserializing a {typeof(T).Name} is not supported.");

test/OrchardCore.Tests/Data/JsonDynamicTests.cs Outdated Show resolved Hide resolved
test/OrchardCore.Tests/Data/JsonDynamicTests.cs Outdated Show resolved Hide resolved
@hyzx86
Copy link
Contributor

hyzx86 commented Jun 26, 2024

A week has passed. Nothing for now, is it mandatory that we use expression ? 🤨

gvkries and others added 2 commits June 26, 2024 18:31
@gvkries
Copy link
Contributor Author

gvkries commented Jun 26, 2024

I don't see the remaining formatting suggestions as mandatory... 🙈

@hyzx86
Copy link
Contributor

hyzx86 commented Jun 26, 2024

Hi @hishamco ,All right, all right, let's merge it quickly.
f81e195dbb51b505036cba00ecfb23a0

@giannik
Copy link
Contributor

giannik commented Jun 26, 2024

merge, merge...

@MikeAlhayek MikeAlhayek enabled auto-merge (squash) June 26, 2024 17:29
@MikeAlhayek
Copy link
Member

@gvkries I enabled auto merge. Hopefully you are done making changes :)

@MikeAlhayek MikeAlhayek merged commit 51eb7b8 into OrchardCMS:main Jun 26, 2024
5 checks passed
@sebastienros
Copy link
Member

@sebastienros if there's any objection regarding expression bodies please let me, because it was a useful feature for single-line methods

I have stated my "personal" preferences and motivations several times already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants