Detect and serialize objects with null prototype; provide shouldJSONSerializeBody hook to override behavior

[trevor-scheer]

This PR refines the body serialization conditional to be more straightforward (rather than a confusing mix of do's and don'ts). Bodies are JSON serialized if:

• It's an array or a plain object
• It has a .toJSON() function on it (with the exception of Buffer which we do not want to stringify)

Lodash utilities are widely used and well-tested, so I went with the isPlainObject implementation they provide in order to correctly detect objects. The existing conditional falls short for objects with a null prototype.

For users desiring to override this behavior, we've provided the new protected method shouldJSONSerializeBody. This method should return a boolean in order to tell RESTDataSource whether or not to call JSON.stringify on the body in question.

Some implementations of fetch support different data types (such as FormData). You're welcome to use those, but it's your responsibility to use libraries that are compatible with each other, i.e. node-fetch is compatible with form-data.

Fixes #67
Fixes #68
Ref: apollographql/apollo-server#5070

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[aeddie-zapidhire]

Looks good to me. Only comment I might possibly make is that https://github.com/sindresorhus/is-plain-obj could be a smidge cleaner and more performant.

[glasser]

What about taking the opposite approach: serialize anything truthy that is not a string or Buffer?

[glasser]

(That's a good fit for our FetcherRequestInit body?: string | Buffer: anything that can't go directly here (ie, not string or Buffer or undefined) gets treated as JSON. So probably "not undefined" rather than "truthy" in my previous suggestion.)

[trevor-scheer]

@glasser that change will fail one of the existing tests (that I fixed in this PR):
does not serialize a request body that is not an object

There's some additional context, particularly in these 2:
apollographql/apollo-server#1316
apollographql/apollo-server#5070

So...if you still think we should go that route I think we'd have to add some error handling around the stringify call and possibly add a hook for handling those errors?

[glasser]

Hmm. Should we consider just explicitly supporting form-data in Fetcher (even though the Apollo Server/Gateway use cases don't need it)? Would require us to have a reasonably portable way of describing a FormData in TypeScript and a reasonable level of confidence that all the important implementations support it with portability between FormData implementations. (My vague memory is of looking into this at some point and deciding that they do mostly support it.)

It just seems odd to introduce a bunch of complexity just in order to ensure we can pass a FormData to an API that is defined as only taking string or Buffer.

[glasser]

i still kinda feel like "anything but string or Buffer" or "anything but string, Buffer, or FormData" would be a better default — the goal of this is to produce something that the Fetch API understands, and so it feels like we should serialize anything other than what Fetch understands.

On the other hand, real Fetch APIs also take other stuff like readable streams, URLSearchParameters, TypedArray, Blob, etc, and perhaps your choice here is better for those things (and we're already moving away from "serialize anything that our Fetcher interface doesn't expect" due to FormData).

So I guess this does seem fine, given that it's just a default people can change.