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

Add pure JSON serialization cheatcodes #5764

Open
Tracked by #3801
vdrg opened this issue Aug 31, 2023 · 2 comments
Open
Tracked by #3801

Add pure JSON serialization cheatcodes #5764

vdrg opened this issue Aug 31, 2023 · 2 comments
Labels
A-cheatcodes Area: cheatcodes T-feature Type: feature T-post-V1 Area: to tackle after V1

Comments

@vdrg
Copy link
Contributor

vdrg commented Aug 31, 2023

Component

Forge

Describe the feature you would like

Hi everyone!

The current approach to handling JSON feels somewhat suboptimal. Working with the id -> json map feels like an extra step, which, in my opinion, isn't entirely necessary. If the JSON serialization methods were made "pure", they could operate directly on a JSON string, which has the advantages of not modifying state and being more intuitive.

Consider the current method:

string memory id = "myObj";
vm.serializeString(id, ".foo", "bar");
vm.serializeString(id, ".hello", "world");
string memory json = vm.serializeUint(id, ".someUint", 123);

With a direct JSON string approach, it could be simplified to:

string memory json = '{ "foo": "bar" }';
json = vm.serializeString(json, ".hello", "world");
json = vm.serializeUint(json, ".someUint", 123);

This method offers more clarity in the way we work with JSON. Also, by the convention used by forge-std, in this case the functions could be declared as pure. Of course this would break a lot of things... an alternative would be to add these as new methods (not ideal as the vm interface is already pretty big).

Thoughts?

Additional context

No response

@vdrg vdrg added the T-feature Type: feature label Aug 31, 2023
@gakonst gakonst added this to Foundry Aug 31, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Aug 31, 2023
@mds1
Copy link
Collaborator

mds1 commented Sep 5, 2023

This does make sense to me and feels simpler.

@odyslam any insight into why we went with the current approach? I feel like I recall expecting it to work in the way suggested here, and can't remember the rationale for the current approach.

Given scope of the change, if we do implement this, my suggestion would be to namespace the cheats behind a _v2 or something and if everyone agrees this is better then at some point there can be a breaking change to remove the old version

@zerosnacks zerosnacks closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 2024
@zerosnacks zerosnacks reopened this Jul 3, 2024
@zerosnacks
Copy link
Member

zerosnacks commented Jul 3, 2024

Reopening, my apologies - misread the proposed suggestion

Adding to larger meta-tracking ticket for JSON / TOML parsing: #3801

@zerosnacks zerosnacks added the A-cheatcodes Area: cheatcodes label Jul 3, 2024
@zerosnacks zerosnacks changed the title Pure json serialization cheatcodes Add pure JSON serialization cheatcodes Jul 3, 2024
@zerosnacks zerosnacks added A-cheatcodes Area: cheatcodes T-feature Type: feature and removed T-feature Type: feature A-cheatcodes Area: cheatcodes labels Jul 5, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@grandizzy grandizzy removed this from the v1.0.0 milestone Oct 8, 2024
@grandizzy grandizzy added the T-post-V1 Area: to tackle after V1 label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes T-feature Type: feature T-post-V1 Area: to tackle after V1
Projects
Status: Todo
Development

No branches or pull requests

4 participants