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

Change the JSON generation method #473

Open
notJoon opened this issue Jan 21, 2025 · 1 comment
Open

Change the JSON generation method #473

notJoon opened this issue Jan 21, 2025 · 1 comment
Assignees

Comments

@notJoon
Copy link
Member

notJoon commented Jan 21, 2025

Description

Functions called via API internally use the gno.land/p/demo/json package to generate JSON. This package operates by directly adding required fields to json.Node objects when creating JSON and then encoding them.

Looking at the current JSON generation approach in our codebase, since required fields are called and generagted within each function as needed, there is a lot of redundancy and many sections with poor readability and maintainability. In my opinition, it would better align with this package's intended usage to create functions that generate json.Node for each type and then assemble them using the builder pattern (supported by the package as json.Builder).

Additionally, there are cases where numeric and boolean values are converted to strings, before output during JSON generation. We could reduce unnecessary operations by outputting these values in their original types without conversion.

Furthermore, when comparing JSON output values in tests, we're currently using direct string comparison, which significantly reduces test maintainability. This issue can be resolved by using the methods provides by JSON package (refer to the `json/nodes.gno file).

Prior Arts

  1. feat(examples): forms #2 gnolang/gno#3524 (comment)
  2. https://github.com/gnoswap-labs/gnoswap/blob/main/contract/r/gnoswap/launchpad/json_builder.gno
@notJoon notJoon self-assigned this Jan 21, 2025
@dongwon8247
Copy link
Member

Note that we might need to change the current usage of the JSON package to Gno's native JSON encoder when it's introduced before mainnet.

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

No branches or pull requests

2 participants