fix: serialize objects with null-prototype and more

[BridgeAR]

The code to detect objects was malformed due to not detecting objects
with direct constructors other than Object.
It was implemented to prevent multipart data to be serialized. This
should be prevented by detecting the multipart header and not
the object type. This adds another safe guard to skip objects that
might not be serializable in the first place.

This is likely a breaking change while still being a bug fix.

Refs: #1316
Refs: #2707
Fixes: apollographql/datasource-rest#68

[apollo-cla]

@BridgeAR: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[BridgeAR]

@glasser PTAL

[glasser]

@BridgeAR yup, this is on my list of PRs to review on a day that I have time to focus on non-obvious things :)

[BridgeAR]

@glasser is there something I can do to help? I am for example happy to describe the issue and the solution closer, if that would help.

[BridgeAR]

@glasser is there a way that I can help to get this looked at again? :-)

[glasser]

Here's the deal. This repository essentially contains two almost entirely separate projects:

• Apollo Server, a project for creating GraphQL servers and linking them to a variety of web frameworks and platforms with an plugin extension library
• apollo-datasource-rest, a caching HTTP client

There's like three lines of code in apollo-server-core that loosely connects them, though in practice that hardcoded integration could actually be replaced by a use of the Apollo Server plugin API.

These projects are both important but the work required to understand and maintain them is more or less disconnected. Learning about all the intricacies of how 8 web frameworks serve GraphQL does not help with learning how to improve a caching HTTP client.

The good news is that the Apollo Server project has been restaffed this year and we're making real progress on shipping incremental improvements to Apollo Server and finally finishing the delayed project of releasing Apollo Server 3, which reduces Apollo Server's external dependencies and makes it much easier to continue to iterate. The other good news is that the Apollo Server team (such as myself) is motivated to continue to maintain and improve apollo-datastore-rest as well. The bad news is that the work we've put in so far to revitalizing Apollo Server doesn't actually directly help apollo-datasource-rest, because they are effectively separate projects. While we (particularly I) do intend to restore apollo-datasource-rest to active maintenance, step one is actually making the time to learn about the project: how it works, what current user concerns are, what the outstanding issues and PRs are, etc. Just merging PRs into a project I don't yet understand wouldn't be particularly helpful for anybody.

Right now we're focused on finishing last year's goals for Apollo Server rather than splitting focus between that and onboarding onto apollo-datastore-rest.

I really do honestly aim to return the other project in this repository to a state of actual maintenance in the next month or two, like we have with Apollo Server. I certainly would not judge any user who reacts to the lack of active maintenance of apollo-datastore-rest by seeking alternatives; I hope to have better a better answer sooner rather than later.

[BridgeAR]

I very well understand that you want to have in-depth insight into the functionality and the issues to better judge upon PRs like this one. The reason for this PR is that I already had a look at that and I am happy to explain the issue further.

It all started with #1316. The problem was that form-data would not properly be passed through. The PR tried to address this by checking the object type to have the global Object object as constructor. This is not ideal for many cases, as pointed out by others. The check was relaxed by 54a8fac and e8e4902 to accept further input shapes but those were no full fixes, since there are lots of other data structures which should be serialized properly. The underlying issue to expect a specific object shape is still in place. Other PRs also tried to address this but without taking the original issue about form-data into account.

This PR addresses the issue in a a way that the author of the original PR has called "a better solution". #1316 (comment) It does that by checking for the form-data header and to skip serialization in that case while serializing all other object types. This is likely the best for everyone: form-data is still possible to be handled appropriately while regular objects do not have to be copied by every user anymore.

The requirement for copying is mainly that some objects in apollo have a null prototype by default and therefore there's no object as constructor. The shape of an object should however not determine if it's serialized or not.

[glasser]

This package has moved to a new repository. I'm closing this PR, but we do plan to spend some time working on RESTDataSource soon; I opened apollographql/datasource-rest#67 to remind us to look at this PR. Sorry for the delay!