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

JSON Replacer Parity #33

Open
Offroaders123 opened this issue Aug 15, 2023 · 0 comments
Open

JSON Replacer Parity #33

Offroaders123 opened this issue Aug 15, 2023 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@Offroaders123
Copy link
Owner

Was thinking about how NBTify is meant to be fairly symmetrical to the JSON namespace, and remembered that neither the SNBT nor binary NBT modules support the replacer callback parameter.

I think I initially skipped over this because I didn't want to worry about supporting it on top of the already complex NBT process, and I didn't want to messify the codebase just to include something initially. I knew I could come back to it again later if it deemed worthy down the line. I think it's something helpful to have in general, for the serialization processes, and it's already in the JSON namespace's API, so I'm going to look into it now.

I was going to just get it all added tonight, because I thought it would be simple and quick to add. But having looked back at the standard lib type definitions, there's more overloads for JSON.stringify() than I remembered. So It's gonna take a little extra to get setup, and I think I'll want to do some additional refactors to the general NBTify API before I try and add these features into the existing stack.

// Standard lib typings for the JSON namespace

interface JSON {
  parse(text: string, reviver?: (this: any, key: string, value: any) => any): any;
  stringify(value: any, replacer?: (this: any, key: string, value: any) => any, space?: string | number): string;
  stringify(value: any, replacer?: (number | string)[] | null, space?: string | number): string;
}

The JSON.stringify() keys selector overload is the one that made me rethink quickly adding this, because I want it to be structurally sound, rather than just patched in real-quick. Anything to do with the serialization steps are definitely important, and even more-so when modifying the user's incoming data. You don't want to unexpectedly mess with that stuff! A lot could go wrong there.

Right now, NBTify just supports the space option with the stringify() function through an options object, which I used instead of a bare-parameter because I wanted to anticipate adding more possible options down the road. That's come to pass though, and I think I'll want to follow the JSON namespace's API instead.

// NBTify's current SNBT API

export declare function parse<T extends RootTag = any>(data: string): T;

export interface StringifyOptions {
    space?: string | number;
}
export declare function stringify<T extends RootTag = any>(data: T | NBTData<T>, options?: StringifyOptions): string;
@Offroaders123 Offroaders123 added the enhancement New feature or request label Aug 15, 2023
@Offroaders123 Offroaders123 self-assigned this Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant