JSON serialization #91
Conversation
| * We shouldn't return `Span<T>`; we should let the caller pass in the buffer to | ||
| fill. This likely also requires `out` parameters to indicate how many bytes | ||
| were written. | ||
| * The APIs returning `ValueTask` should probably return `Task` |
There was a problem hiding this comment.
Also add, we should re-order the parameters so that the object is first (as source) and the destination is second.
For these two APIs, PipeWriter and Stream should be second.
public static ValueTask ToJsonAsync(this PipeWriter writer, object value, JsonConverterOptions options = null, CancellationToken cancellationToken = default);
public static ValueTask ToJsonsync(this Stream stream, object value, JsonConverterOptions options = null, CancellationToken cancellationToken = default);
There was a problem hiding this comment.
What is the dependency graph going to look like for these?
This makes System.Text.Json to depend on System.IO.Pipelines. System.Text.Json is inbox, System.IO.Pipelines is not inbox today.
There was a problem hiding this comment.
What is the dependency graph going to look like for these?
That is a great question. We need to figure out the layering. If System.Text.Json depends on System.IO.Pipelines (which it would have to if we were to provide PipeReader/Writer overloads), it would force S.IO.Pipelines to be inbox. Is that even feasible?
There was a problem hiding this comment.
You mean be part of Microsoft.NETCore.App? I think it should be in box and I was actually going to suggest it be in box outside of this conversation.
There was a problem hiding this comment.
Also on the System.Text.Json packaging we will likely need 3 build flavors:
- NETCore.App inbox
- .NET Framework package
- netstandard package
Today we do 1 and 3.
The reason for 2 is the performance of using System.Reflection instead of IL.Emit causes the serializer to be much slower.
There was a problem hiding this comment.
What will .NET Framework use -- the source code only package? IL Emit is not supported by netstandard2.0.
There was a problem hiding this comment.
What will .NET Framework use -- the source code only package?
Yep. Our current plan is that only netcoreapp gets compiled assets for System.Text.Json
There was a problem hiding this comment.
OK so lets settle on this. System.IO.Pipelines will be in box in 3.0. We can file an issue to raise awareness in case there's pushback.
There was a problem hiding this comment.
Even if we put pipelines in box, I don't think the JSON component should depend on pipelines. Pipelines could depend on JSON though.
There was a problem hiding this comment.
No that’s wrong. Let’s discuss this outside of GitHub through. Pipelines should not become a dumping ground for all integrations with it.
No description provided.