Apollo-client: `addTypename` does not feel like a great API

[wmertens]

• I have globally unique ids (on __id), and so I don't need __typename added on my queries. Apollo-client adds these by default, which is surprising. Surprising behavior in libraries is not great, especially if it eats resources.
• AFAICT addTypename is only needed for the default dataIdFromObject, so if you provide your own function, the default setting of addTypename is not even useful.
• I don't know the history of the queryTransformer, I presume that addTypename is the only one deemed useful, but it still feels somewhat arbitrary
• It is not clear that there is an interaction between the default dataIdFromObject and the addTypename setting. The default dataIdFromObject doesn't even protect from a missing __typename.

So what I propose is to set addTypename to false by default, make __id be a de-facto "pinkie-promise-globally-unique-id", and to make the default dataIdFromObject be something that either returns __id or else the __typename + id if both are not null.

Something like o => o.__id != null ? o.__id : (o.__typename && o.id != null ? o.__typename + o.id : undefined)

[helfer]

@wmertens I'm not sure what point you're trying to make? Is it that you don't like __typename, but are okay with __id (which isn't part of the spec).

Regardless, we need the typename to disambiguate between fragments of different types, which is why it's added to the query by default.

I think it's perfectly fine that apollo client applies predictable transformations to the query, the only thing I think we should still change is that the result returned from apollo client should be for the original query, not the transformed one.

I don't think we should have a default dataIdFromObject until __id is officially in the spec.

PS: I would also strongly advise against adding fields that start with __ to your schema. Those should be reserved for meta fields that are part of graphql.

[wmertens]

Hmm. My problem with __typename is indeed that it shows up in my query
results and that it adds fairly significant overhead, even in cases where
it is not needed.

[helfer]

How does it add significant overhead? You mean just because the result is longer? I would venture a guess that with compression that overhead is negligible. Do you have any numbers that show otherwise?

[stubailo]

It is not clear that there is an interaction between the default dataIdFromObject and the addTypename setting. The default dataIdFromObject doesn't even protect from a missing __typename.

Apollo Client doesn't have a default dataIdFromObject at all.

[wmertens]

@helfer yes, the result is longer, and it can be a significant portion of the payload when getting arrays of small objects (e.g. a bunch of numbers). Even if gzip removes that in transit, they are still present in the resulting objects in the recipient.

How about not adding __typename at all, and instead requiring it to be added into the query by the user when it is needed? That way, there is no magic and it is only added when necessary.

[helfer]

In my opinion that would be worse, because then you'd have to add it manually everywhere. If your only concern is with the result having fields you didn't ask for then it's only a temporary problem, because we're going to make it disappear in future versions.

[wmertens]

The way I understand it, __typename is only needed when you are resolving
fragments or unions, right? So in all other cases, __typename is not needed?

I like it when libraries are not too magic, so personally I would probably
be happy to add __typename where required. Note that I don't really use
fragments yet.

OTOH if you could make the __typename insertion only happen when it is
actually needed, I would be ok with that.

At the very least, how about an addTypename option in the graphql decorator
so you can disable it where it adds overhead?

[helfer]

Yes, right now it's used only for fragment matching, but many people also use it for dataIdFromObject.

I like it when libraries are not too magic, so personally I would probably
be happy to add __typename where required. Note that I don't really use
fragments yet.

That's exactly what we have the addTypename option for. Feel free to set it to false and add __typename yourself where necessary.

As a first step, we're going to make sure that __typename isn't added in the result of Apollo Client if the query didn't ask for it, but it will still request it every time. After that, we may optimize it a little more by only adding __typename when it's really needed.

If you want to do something to make things move faster in that direction, you could help by finding out which parts of the code we would have to modify in order to not return __typename if it wasn't asked for. I suspect that all we'd have to do is call the transform right before making the query, instead of calling it as we receive the document. 🙂

[calebmer]

Interesting concerns, but I don’t think we will ever remove this API. It is just too useful in many different circumstances such as:

1. When building ids for users who don’t have a strong __id convention like @wmertens.
2. For resolving fragment spreads in a query.
3. For determining data identity when there is no id. For example if { a { b } } first returned data for a with __typename X and then later returned data with __typename Y it may be useful for us in the future to throw away all the old data in the store as we can reasonably guess that the data of the identity changed.

Furthermore, I really don’t think the cost of __typename is that high. After gzip the difference is minimal. It’s definitely not the worst problem we’re seeing in Apollo Client usage. If there’s data that suggests otherwise, we would love to see it :+1:

Finally you don’t even have to use the addTypename API. You could just set it to false and intelligently add __typenames wherever needed in your app code.

So while maybe the API doesn’t feel right it still has some valid use cases and is not required. Thanks so much for your feedback @wmertens. If there’s anything you want to add let us know :+1: