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.parse[@@customMatcher]? #310

Open
Jack-Works opened this issue Nov 30, 2023 · 6 comments
Open

JSON.parse[@@customMatcher]? #310

Jack-Works opened this issue Nov 30, 2023 · 6 comments

Comments

@Jack-Works
Copy link
Member

because tryParse isn't a thing now, I wonder if we can have JSON.parse[@@customMatcher] to allow matches JSON structure inside a string.

({data: '{"x": 1}'}) is { data: JSON.parse({ x: 1 }) }

or if it should be "reflective", it might be on stringify

({data: '{"x": 1}'}) is { data: JSON.stringify({ x: 1 }) }
@ljharb
Copy link
Member

ljharb commented Nov 30, 2023

The latter seems like it'd be really brittle with various formatting options, and the former seems pretty expensive. Even if we could make it make sense, I'm not sure we'd want to encourage people to do this - also, we should definitely never encourage anyone to hand-write a JSON string.

@Jack-Works
Copy link
Member Author

The latter seems like it'd be really brittle with various formatting options, and the former seems pretty expensive. Even if we could make it make sense, I'm not sure we'd want to encourage people to do this - also, we should definitely never encourage anyone to hand-write a JSON string.

The example is just to clarify what the input is. In real life it might be like this:

const file = await readFile()
const config = match (file) {
    JSON.parse({ let config and isValidConfig }): config;
    Yaml.parse({ let config and isValidYamlConfig }): config;
    default: throw new Error()
}

@ljharb
Copy link
Member

ljharb commented Nov 30, 2023

that seems like if would attempt to YAML-parse JSON that didn't match the expected schema - i'm not sure this can be shoehorned into pattern matching cleanly tbh.

@tabatkins
Copy link
Collaborator

Just for readability, I don't think attaching the custom matcher to .parse() is the way to go. The usual pattern is that extractors should be readable as "constructor, but in reverse" - the pattern in the args should match, roughly, what would have gone into the function call for that argument originally. JSON.parse() takes a string, so that suggests the JSON.parse custom matcher exposes a string as its only iterable value.

I could see installing it on JSON itself, tho. match(someJson) { JSON( {let foo} ): ...; } feels better, to me.

@Jack-Works
Copy link
Member Author

I could see installing it on JSON itself, tho. match(someJson) { JSON( {let foo} ): ...; } feels better, to me.

Yes, but iirc @rbuckton doesn't like it because new JSON(object) isn't a thing, so you should not install the customMatcher on the JSON object.

@ljharb
Copy link
Member

ljharb commented Dec 12, 2023

While i don’t think we should restrict ourselves to only see matching as the inverse of new - it’s simply not that - a JSON matcher seems the same as tryParse to me, and unless the committee is convinced of that method’s motivations i suspect they wouldn’t be convinced of this matcher’s motivations.

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

3 participants