hapijs / hapi

The Simple, Secure Framework Developers Trust
https://hapi.dev
Other
14.63k stars 1.34k forks source link

Allow replacing JSON.stringify #4450

Open joshkel opened 1 year ago

joshkel commented 1 year ago

Support plan

Context

What problem are you trying to solve?

In performance tests, we've found that calling JSON.stringify on large responses is a significant bottleneck. We're experimenting with alternatives such as fast-json-stringify.

Do you have a new or modified API suggestion to solve the problem?

I saw in https://github.com/hapijs/hapi/pull/3014 that there was a brief discussion about using an alternative to JSON.stringify. I completely understand the desire expressed there to not add dependencies to the Hapi core, but it like there could be some value in adding an option to use a function other than JSON.stringify, so that users can use alternatives themselves if they want.

Suggested API: Add route.options.stringify or route.options.jsonStringify, as a function that takes an arbitrary object and returns a string. If this option is present, and if a response is a normal (i.e., success, not Boom) response of 'plain' type (such that it would normally go through JSON.stringify), then it's invoked instead of JSON.stringify, and route.options.json is ignored.

(Ignoring this new option for Boom error responses simplifies usage with alternatives like fast-json-stringify, which expect to know the schema of the object(s) they're used with.)

Marsup commented 1 year ago

I know it's how some other frameworks deal with it, but I'm questioning whether it's even a good practice that should be encouraged. Shouldn't large payloads be streamed instead of fully serialized?

kanongil commented 1 year ago

Yeah, at the point where you have a performance issue with the built-in handling, you might be better served with other approaches.

If someone wants to make a generic solution, I expect a new plugin can be made that exposes a dedicated (possibly streaming enabled) h.json() handler. Either using pluggable stringifiers, or directly using fast-json-stringify or similar.

FYI, streaming is probably not viable in a generic sense, but might be viable for specific scenarios. Eg. a for long array of objects.

devinivy commented 1 year ago

It's a great idea to have a straightforward way of using fast-json-stringify with hapi. I agree with Gil— experimentation around this is what the plugin system is so useful for, and I think the ideal way of handling this would be to implement a custom response type h.json() similar to how vision's h.view() or inert's h.file() work. This would also have a straightforward way of hooking into settings on the route config, which seems useful in the case of fast-json-stringify which requires a json schema per output type.

alexey-sh commented 6 months ago

It looks like nobody uses hapi in heavy apps(because it doesn't provide this simple perf tuning). So maybe it makes sense to focus on simplicity and close this issue as 'won't fix'. Just to be a developer friendly and not to give false hope that it will ever be fixed.