Skip to content

zon: add zonStringify to override the default behavior + json.dynamic… #23619

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tiawl
Copy link

@tiawl tiawl commented Apr 20, 2025

Hi,

This PR adds the possibility to override the ZON serialization default behavior if a struct or union declares a method pub fn zonStringify(self: *@This(), zon_serializer: anytype, options: std.zon.stringify.ValueOptions) !void.
It also adds a naive implementation of zonStringify() for the std.json.dynamic.Value union (which is useful for JSON to ZON conversion).

Thank you.

@alexrp
Copy link
Member

alexrp commented Apr 20, 2025

cc @MasonRemaley

@MasonRemaley
Copy link
Contributor

MasonRemaley commented Apr 20, 2025

Thanks for the PR!

I was hoping to avoid the method callback pattern by providing "Serializer" as a lower level API that gives you more control over the output by allowing you to serialize a type piece by piece, since the method callback pattern only works when you can edit the type in question to add the callback method.

I can imagine some scenarios where just using Serializer isn't a satisfactory solution (eg customizing a deeply nested type), but I'm not sure whether they come up in practice or not.

I'm guessing one of these scenarios did come up in practice and that's what motivated this PR. If so, would you mind sharing the use case?

I want to look at it to make sure there isn't a way to solve it with Serializer, or to extend Serializer to make that possible. If there isn't then I'll consider my attempt to avoid the callback a failed experiment. :)

@tiawl
Copy link
Author

tiawl commented Apr 20, 2025

Hi,

I do not see the "Serializer" and the method callback pattern as concurrent solutions. It is maybe just serving different usecases.
I see the method callback pattern as a possibility:

  • for a struct or union, to have responsability to manage ZON serialization of its own instances,
  • for the user, to extend the std.zon.stringify.serialize() functions with his/her own types without diving deeply into the API

For the "Serializer", this is the right solution when higher level API does not provide a suitable solution.

I'm guessing one of these scenarios did come up in practice and that's what motivated this PR. If so, would you mind sharing the use case?

Yeah, for sure. But you probably already guessed it with the PR content: I was playing with zig and a big JSON file, searching how to convert it into ZON format with the new std.zon API. So I just wrote something really simple like this:

const std = @import ("std");

test "json2zon" {
    var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
    defer arena.deinit();
    const allocator = arena.allocator();

    const read = try std.fs.cwd().readFileAlloc(allocator, "/path/to/my/file.json", std.math.maxInt(usize));

    var scanner = std.json.Scanner.initCompleteInput(allocator, read);

    var diag: std.json.Diagnostics = .{};
    scanner.enableDiagnostics(&diag);

    const json_value = std.json.parseFromTokenSourceLeaky(std.json.Value, allocator, &scanner, .{
        .ignore_unknown_fields = true,
    }) catch {
        std.debug.panic("Potential error at line {} and column {}", .{
            diag.getLine(), diag.getColumn(),
        });
    };

    var buffer = std.ArrayList(u8).init(allocator);
    const writer = buffer.writer();
    try std.zon.stringify.serializeArbitraryDepth(json_value, .{}, writer);

    try buffer.append(0);
    const source = buffer.items[0 .. buffer.items.len - 1 :0];

    try std.fs.cwd().writeFile(.{
        .sub_path = "output.zon",
        .data = source,
    });
}

And you are right, it fails because:

  • std.json.dynamic.Value is a nested type
  • std.zon.stringify.canSerializeTypeInner() panicked => std.json.dynamic.Value.Array and std.json.dynamic.Value.Object have an allocator field which implements the std.mem.Allocator interface (and it comes with its anyopaque pointer).

I maybe can achieve this with the "Serializer". I have to admit that I did not give it a try because it was really quick to make my unambitious usecase works with these small changes into the std lib.

@MasonRemaley
Copy link
Contributor

MasonRemaley commented Apr 24, 2025

Those are good points. I think the features you implemented here provide a lot of value, and my concern doesn't really make sense since we already accept this pattern elsewhere in standard library anyway (std.json, std.fmt.)

This probably needs some tests but otherwise it LGTM!

@tiawl
Copy link
Author

tiawl commented Apr 24, 2025

This probably needs some tests but otherwise it LGTM!

I will go for this during this week-end or the next week. I will ping you when done. Thanks for your feedback.

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

Successfully merging this pull request may close these issues.

3 participants