JSON.parse[@@customMatcher]? #310

Activity

1. ljharb commented on Nov 30, 2023

   

   ljharb

   on Nov 30, 2023

   Member

   More actions

   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.
2. Jack-Works commented on Nov 30, 2023

   

   Jack-Works

   on Nov 30, 2023

   MemberAuthor

   More actions

   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()
   }

   👀React with 👀1Reacted by Jordan Harband
3. ljharb commented on Nov 30, 2023

   

   ljharb

   on Nov 30, 2023

   Member

   More actions

   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.
4. tabatkins commented on Dec 11, 2023

   

   tabatkins

   on Dec 11, 2023

   Collaborator

   More actions

   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.

   👍React with 👍1Reacted by Bo Lingen
5. Jack-Works commented on Dec 12, 2023

   

   Jack-Works

   on Dec 12, 2023

   MemberAuthor

   More actions

   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.
6. ljharb commented on Dec 12, 2023

   

   ljharb

   on Dec 12, 2023

   Member

   More actions

   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.