expressjs / body-parser

Node.js body parsing middleware
MIT License
5.45k stars 727 forks source link

Custom JSON parser #513

Closed vbelius closed 9 months ago

vbelius commented 9 months ago

To solve the need for custom JSON parses, like in issue [#278] and issue [#478], I've added an option for a custom JSON parser. This is a simpler implementation with more limited scope than the stale [#282] PR.

I was a bit unsure on how to best handle the custom error handling/normalization built into the existing JSON parser. I opted for skipping this when supplied with a custom parser, to allow custom parsers to throw their own errors without manipulation, mostly since the normalizeJsonSyntaxError method removes all keys except message and stack, which might be set on custom and errors and might be useful for those using custom parsers.

      // Only normalize errors if using default parser
      // Custom parsers might throw custom errors
      if (parser === JSON.parse) {
        throw normalizeJsonSyntaxError(e, {
          message: e.message,
          stack: e.stack
        })
      } else {
        throw e
      }

What do you think @dougwilson? Do you have any other feedback that might help get this merged? Thank you.

PS. My personal motivation to implement this feature is to enable the use of json-bigint for parsing json.

dougwilson commented 9 months ago

Hello, and while your PR is appreciated, the intention is that we do not want to add a parser option to eqch and every type, thus having instead a single generic body parser where you can define a paerser that would do anything you want for any type. I'm sorry if you missed that somewhere, as we have closed these prior.

vbelius commented 9 months ago

@dougwilson Could I argue that since it's been 7 years without any progress on this issue, and 10 years without any progress on #22, wouldn't an intermediary solution be more beneficial to no solution? It's a very simple PR, not adding lots of code or complexity to the project. It could also be argued for us casual JSON users with, special needs, that it would easier to provide a custom option rather than implement a full custom parser?

I hope you find my contemplations in good spirit, thank you for all your awesome OSS work :)

dougwilson commented 9 months ago

Hello, I would argue perhaps if you like to help, please help make the generic parser... This is adding complexity to the JSON parser that is unnecessary, just think how you are adding the different code paths to avoid the JSON.parse error munging and making the json parser now work two different ways.

I can explain the gap in development, but it is not relevant here and unrelated to trying to add a feature. Please if you are going to contribute to a project, it would be nice to respect the design decisions of the project.