Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation of local resolvers makes adopting GQL more difficult #7072

Closed
vigie opened this issue Sep 24, 2020 · 29 comments
Closed

Deprecation of local resolvers makes adopting GQL more difficult #7072

vigie opened this issue Sep 24, 2020 · 29 comments

Comments

@vigie
Copy link

vigie commented Sep 24, 2020

Apologies if this has been raised elsewhere, but I spent quite some time searching and couldn't find anything coming at this with quite the same angle I want to use.

I want to argue that deprecating both apollo-link-state and local resolvers from core simultaneously does serious damage to the powerful legacy API adoption story that Apollo and GQL has thus far been able to boast. By "legacy API adoption story" I mean the ability to start using Apollo without incurring tech debt in the client prior to a GQL server becoming available. This is such a valuable story because it empowers front end devs, who are often the main advocates for GQL, to start to demonstrate value to the entire cross-functional team without having immediate dependencies on back-end buy-in.

Noticing that local resolvers had been built into core in 2.5, I decided the time was right to adopt Apollo client as our primary state management solution. In part I was spurred on by this excellent blog post about successful adoption at Trello. I built our solution on a v3 RC. When v3 officially came out it was quite a shock to see the deprecation of the local resolvers feature, so soon after its promotion into core.

The reason I find local resolvers such a powerful and preferential solution is that it allows me to build my client (almost entirely) agnostic of where the graphql service exists. Today, it happens to be in the browser alongside the UI code consuming it, but at some point in the future we will have a real service running across the wire. When that point comes I expect to need to make almost no changes to my client in order to consume it (removing some @client directives and adding the GQL service URL should be about it). Compare this with the disruption that would be caused if the client had originally been written to use the rest link (as pointed out in the above blog post) or the new type policies strategy being advocated.

(The potential to use most of the resolver code in a real node GQL service is also significant but incidental here.)

On type policies - I can kind of see how they could be used to get basically equivalent behavior, based on the suggestion here apollographql/apollo-feature-requests#383, but it's less clear to me how this would look at scale. That is, what are the patterns to follow to migrate a local resolver GQL service solution over to a new type policies solution? I appreciate that there is another issue open tracking outstanding documentation #6711 - I do hope someone will write a blog post or doc chapter to illustrate this soon.

I'll finish with a few options that occur to me for ways forward, for consideration. The aim is to provide developers such as myself who have adopted Apollo on the client prior to the availability of a GQL server to continue to do so using version 3 and beyond without incurring technical debt in the client.

  • Do not deprecate both local resolvers from core and apollo-link-state.
  • Sufficiently reduce the size of the gql execution layer so that schema link becomes a viable alternative
  • Publish some kind of migration guide/best practice doc that shows that my fears about converting my local resolver implementation over to a type policies one are unfounded ;-)

Thanks for all of your time, work and thoughts.

@MarMun
Copy link

MarMun commented Sep 25, 2020

The reason I find local resolvers such a powerful and preferential solution is that it allows me to build my client (almost entirely) agnostic of where the graphql service exists.

I agree. My current (and still shaky) understanding of the new paradigm (ReactiveVar / TypePolicies) makes me bleed data access layer knowledge (in my case: apollo client) into the presentation layer.

Namely: My presentation layer needs to know if data is local or remote because the way to issue a mutation is different (gql-mutation vs function call).

@benjamn
Copy link
Member

benjamn commented Sep 25, 2020

@vigie If you're referring to ApolloLink when you talk about the local link, I can reassure you that we have no plans to deprecate ApolloLink. It's still the best way to hook your client up to arbitrary APIs (GraphQL or otherwise), which is a big part of the legacy API adoption story, as you put it.

Removing local resolvers is something we'd like to do at some point, but we realize we don't have a full replacement yet, so that deprecation and removal is still a ways off.

@vigie
Copy link
Author

vigie commented Sep 25, 2020

@vigie If you're referring to ApolloLink when you talk about the local link, I can reassure you that we have no plans to deprecate ApolloLink. It's still the best way to hook your client up to arbitrary APIs (GraphQL or otherwise), which is a big part of the legacy API adoption story, as you put it.

Removing local resolvers is something we'd like to do at some point, but we realize we don't have a full replacement yet, so that deprecation and removal is still a ways off.

I was referring specifically to apollo-link-state (sorry, I think I called it "local link" in my original description, corrected). It is marked as deprecated - that's what I mean by this simultaneous deprecation leaving nowhere for this use case to live.

@vigie
Copy link
Author

vigie commented Sep 28, 2020

Let me try to boil this down a bit: after 2.5 the recommendation for everyone using local resolvers was to abandon newly deprecated apollo-link-state and use the functionality directly from core. Then in v3 local resolvers are deprecated from core, leaving no supported home for anyone who wants to stay with local resolvers for the reasons I give above.

So my concrete ask in this issue is for the Apollo team to, at a minimum, consider reversing the decision to drop support for/deprecate apollo-link-state, in order to provide a supported home for this important use case.

@dmitry
Copy link

dmitry commented Oct 8, 2020

@vigie have you tried to look into "Local-only fields" and replace local resolvers with them? Seems like interface more or less similar, should be replaceable. In case of mutations you have to use reactive variables.

@nfriend
Copy link

nfriend commented Oct 15, 2020

Namely: My presentation layer needs to know if data is local or remote because the way to issue a mutation is different (gql-mutation vs function call).

Agreed! This is the biggest loss, in my opinion. Local resolvers made intuitive sense to me because my components could read and write data without caring whether data is local or remote.

@vigie
Copy link
Author

vigie commented Oct 15, 2020

@vigie have you tried to look into "Local-only fields" and replace local resolvers with them? Seems like interface more or less similar, should be replaceable. In case of mutations you have to use reactive variables.

I would argue that the synchronous nature of the cache means the interface is really rather different, but I'm still somewhat unfamiliar here and I've yet to see a concrete example of how such a rewrite would look, perhaps things aren't as bad as I fear.

But the broader point is that any difference is unfortunate - the GQL service can no longer be moved across the network without disruption to the client.

@fjmorant
Copy link

fjmorant commented Nov 4, 2020

With local resolvers, it was a lot easier to abstract whatever source of data you were using in them and combine them in a single source of truth for the UI, even you could transform graphql data to accommodate it for the UI and cache it in a very abstract way.

I have tried local-only fields but they are very limited in my opinion

@borekb
Copy link

borekb commented Nov 10, 2020

Just going to link to a similar / related question: Trying to understand AC3 vs. AC2.5 state management model #6994

@mbret
Copy link

mbret commented Nov 18, 2020

I have been struggling for couple of weeks now to understand the reason behind reactive variable and field policy. I cannot have a clear unified architecture and my code is much less debuggable. Rather than having one source of truth and one way of query/mutate my state (local + remote) I have to have different logic in my code to deal with local state. https://github.com/apollographql/apollo-client/discussions/6994 summarize it well. Beside reactive variable are not persistable the same way as the state and are a pain to debug. Direct change to the cache is generally harder to debug. This is why having an abstracted layer with local mutation/query through resolvers was a good thing. Would be nice to have some insight about the reason from the team. Especially about reactive variable and why they exist in the first place. I haven't found any explanation about this in the official doc

@dsonck92
Copy link

I'll condense a bit my thoughts from #6994. Following is how I currently play with these reactive variables and policies.

So, I come from a perspective where I migrated a work project specifically to Apollo 3 to play with these field policies and reactive variables. My use case was simple (as far as I currently use it):

  • I want to have a selected items list client side (no need to constantly poke the server with this info)
  • I want to have this selected state available from useQuery for individual items. (Partly because I go several layers into components, and passing through this selection state is not great. If you're not careful, a lot of rerenders will happen)
  • Ideally I also want to have access to the full list, and if it's bound to existing objects, the better (imagine, you can loop through the list as if it were a remote endpoint, and use type information to check which kind of object was selected (we have multiple))
  • Lastly, I want some functionality like useFragment. (We have a rather large tree of objects that is given by the backend, which does correspond to the frontend. But it would be cumbersome to pass in all the data subselections down layers and layers of components. I tried using {children} at some point, but that does not cleanly separate components)

So I looked into reactive variables. Well, gladly there's an example for this selection use case. So it turns out that it's relatively straight forward to use this reactive variable. I could just use filter or findIndex to imitate the old Redux selection logic (essentially creating a "selected" field with an index where -1 means unselected. Yes some legacy feature, could have been a boolean). Now, I did look into the code for these reactive variables, and they seem to be cache aware. So if they get called inside a read function, they will "bind" to this read function, triggering it to update each time it changes. This is good to know as I was wondering how it would work when things change, as on the surface, it simply looks as if you call the value once. But this is why reactive variables are part of the in-memory cache, they are in-memory cache aware.

Considering this. I could essentially implement the first 3 options. Great! And with the help of toReference, I could also enhance the selection to return the actual selected objects. Which is the better part because there are some different ways selected objects need to be handled. So now it becomes possible to query the selection and use fragments to extract the proper data.

Now for the last point. Apparently this use case is also already given. So it was as simple as making a field for the Query type. Looking at the variables given and return a reference to the correct real object. Something akin to:

someObject(_, {args, toReference}) {
  return toReference({
    __typename: 'SomeObject',
    id: args.id,
  }
}

Now this is useful, because in a component I can depend on a query with a simple id. If the cache has this object (which it does because a huge tree is fetched first time), then it will simply get a subset of the data. If things change, only the relevant parts will get an update. This was one of my main points. Now, granted, I would have preferred a solution like useFragment which would essentially do the same: you select bits of information you need based on an id, much like read/writeFragment. But that doesn't exist (yet).

Considering this approach, I feel like the reactive variables are here to help with pure local state, much like the policies. Policies help the client to shape its local usages (the server can (and probably should) provide the "fetch one object", but if the client then spams this query over and over because a lot of objects suddenly want this data, that's bad. With this example policy, the client can reuse the data from the larger query and possibly refetch later if time comes to do so.) From some design decisions over time, I also see this as the original idea behind local resolvers. I don't think they were ever intended to be used for custom fetching. Just looking at the need to put @client to get to local resolvers points to "this is client side only". So I feel like some users may have used an API in a less than ideal manner, at least in the eyes of apollo devs, and now start to see the mistake. Now, I can see the appeal for developing locally first with client side only and using GraphQL from scratch and have no real opinion on whether this should or should not be easily possible. Though personally, I find setting up a server easy enough not to bother development without a server.

I hope this may shed some light. Although I'm not an apollo dev, though I tend to seek info and reason about their api to get most out of it, which I believe I've done quite well thus far.

@buzinas
Copy link

buzinas commented Mar 2, 2021

Our story at Close is basically the same as the author of this issue and identical to Trello's. While the BE was slowly moving towards GraphQL, we wanted to move faster on the FE and started creating local resolvers for retrieving objects from our REST API that didn't have a GraphQL counterpart. When we saw the deprecation notice a couple of months ago, we kinda got lost, because we invested our entire state machine in something that was deprecated 0.5 version after it was introduced and there isn't really a replacement for our use case.

Although I really liked reactive var and type policies for managing local state, the fact that they're synchronous (I know there is a hack, but it's still field by field, it doesn't scale) make them not a good fit for our needs.

What are our possibilities?

@vigie
Copy link
Author

vigie commented Mar 5, 2021

I don't think they were ever intended to be used for custom fetching. Just looking at the need to put @client to get to local resolvers points to "this is client side only". So I feel like some users may have used an API in a less than ideal manner, at least in the eyes of apollo devs, and now start to see the mistake.

That's an interesting theory, would love to get an Apollo maintainer's take on this.

I don't have any specific links to hand, but I do definitely recall being highly motivated at the beginning of my GQL journey a few years ago by blogs/articles effectively stating: "...and look: you can get started with Apollo client even before your backend team is ready, via local resolvers...". Or did I dream that?

A quick search and removal of @client in my source code is entirely trivial as the corresponding GQL server comes online. But stack that against the changes that would be needed to swap from reactive vars/field policies to queries...

@vigie
Copy link
Author

vigie commented Mar 5, 2021

Thanks to all for the engagement on this issue so far. For those that share my concern, I'm interested to know what you see as the best way forward.

I see three possibilities. Here I rank them in order of my own preference, favorite first.

What would your ranking be?

  1. graphql shrinks sufficiently to make running a full GQL service in the browser (in production), probably via apollo-link-schema, acceptable. This is my preference since at that point it all just becomes true, isomorphic GQL and there are no "client side only quirks".
  2. resuscitate apollo-link-state and maintain it as a community link
  3. Petition Apollo maintainers to reverse their deprecation decision.

@nadav-dav
Copy link

Hey guys!
Just getting into this conversation, and I am not sure about the state of this development, but I'd love to help.

I've also struggled with the deprecation of the local resolvers ability, and I think I managed to figure out some sort of middle ground.
I've create a small PoC repo here: https://github.com/nadav-dav/apollo-smart-query

I tried to mimic the current makeVar() behavior, that is currently in the documentation, but also allowing local resolvers that has much more familiar API structure.

WDYT?

@vigie
Copy link
Author

vigie commented Mar 12, 2021

Hi @nadav-dav I don't think this solves anything - you're still using local resolvers, which has been deprecated.

@veeramarni
Copy link

veeramarni commented Mar 15, 2021

I agree, resolvers in state management make few things easy and I would like to see them not deprecated as most of us already using them in production.

@nadav-dav
Copy link

@vigie My perspective is similar to @veeramarni on that, the local resolvers makes sense and reduces the amount of concepts you need to understand when coming into GraphQL / Apollo.
I mean, from an onboarding for new developers perspective it makes sense so keep few core concepts and simplify them as much as possible. That's why i've decided to implement this as local resolvers despite the deprecation, rather than implementing it via the cache (and oh god I tried to make it simple and reducer like, but it was a bit of a rabbit hole).

WDYT? Do you believe my solution has viable aspects to it?

@vigie
Copy link
Author

vigie commented Mar 16, 2021

@nadav-dav perhaps I'm missing something, but this issue is about the deprecation of local resolves themselves and the subsequent loss of a significant developer benefit - that of being able to write component code that does not need to know or care whether the resolvers implementing their queries are client or server side.

Your POC still uses local resolvers, so it does not appear to be a suggestion on how to move forward, other than an appeal to remove the deprecation.

The implementation detail of those resolvers - the fact that you chose to use reactive vars inside them - is not relevant.

@nadav-dav
Copy link

@vigie you are correct, my intention is to allow moving forward by implementing the progress that is local state management using the graph, while keeping the simplicity and familiarity of resolvers. best of both worlds :)

Maybe I am missing something, but to my understanding the local resolvers are deprecated because Apollo wants to allow local state management, and the deprecation will drive developers to use the cache method instead. I am addressing the idea that is stated in this issue, that local resolvers should not be deprecated since they make sense.

@jayy-lmao
Copy link

jayy-lmao commented Mar 30, 2021

Lastly, I want some functionality like useFragment. (We have a rather large tree of objects that is given by the backend, which does correspond to the frontend. But it would be cumbersome to pass in all the data subselections down layers and layers of components. I tried using {children} at some point, but that does not cleanly separate components)

[Note: don't do this]

@dsonck92 at my work we're also trying to use something a bit like a useFragment.

Currently our implementation is just:

/* eslint-disable import/prefer-default-export */
import { useApolloClient } from '@apollo/client';
import { useState, useEffect } from 'react';
import deepEqual from 'fast-deep-equal';

export const useFragment = (
  options,
  optimistic,
) => {
  const client = useApolloClient();
  const cacheData = client?.cache?.data;

  const [loading, setLoading] = useState(true);
  const [data, setData] = useState(null);

  useEffect(
    () => {
      setLoading(true);
      const newData = client.readFragment(
        options,
        optimistic,
      );
      if (!deepEqual(newData, data)) {
        setData(newData);
      }
      setLoading(false);
    },
    /* eslint-disable-next-line react-hooks/exhaustive-deps */
    [
      cacheData,
      options,
      optimistic,
    ],
  );

  return { loading, data };
};

It's not going to win any prizes, but I'm thinking of investigating if there's any better way to observe updates on the cache. Also open to new ideas.

[edit: do not use this with SSR, the first render will not run the useEffect ]

[EDIT: appears this only updates with client.writeFragment and not with queries updating the state]

@nweajoseph
Copy link

yikes, what a scary revelation. i could never have implemented a client using graphql without local resolvers. keeping the frontend agnostic about its datasource was the only leverage i had to introduce graphql to product teams that had never used it. if local resolvers go away and only leave reactive var's, so that useMutation can only talk to a backend, i don't think i could ever use apollo client again. that would necessitate a graphql backend being available simultaneously for any non-read features.

@vigie vigie changed the title Deprecation of local resolvers is damaging to the legacy API adoption story. Deprecation of local resolvers makes adopting GQL more difficult May 24, 2021
@benjamn
Copy link
Member

benjamn commented Jun 8, 2021

Thanks everyone for the constructive and insightful discussion! Sorry to keep you waiting.

Here's a recent comment of mine giving a sketch of the useFragment API we're considering. The initial implementation looks very much like what @jayy-lmao shared above. In short, we're sold on useFragment for AC4. Hope that's good news.

The other good news, more directly relevant to the discussion here: #8189 (comment)

I hope this counts as picking option 3 from the list in #7072 (comment), and then skipping the petitioning part. You already did that, above! I see no reason we can't continue supporting an @apollo/client/link/local-resolvers link package, similar in spirit to apollo-link-state. It just doesn't need to be baked into the ApolloClient core code.

keeping the frontend agnostic about its datasource was the only leverage i had

@nweajoseph This will still be true if we release local resolvers back into the link chain in AC4. In fact, since the link chain represents a network abstraction for Apollo Client, I honestly think a link is the best place to implement local resolvers that mock server data or fetch it from different sources. One way or another, that usage of local resolvers will always be possible, rest assured!

@smikula
Copy link

smikula commented Jun 11, 2021

It's not going to win any prizes, but I'm thinking of investigating if there's any better way to observe updates on the cache. Also open to new ideas.

Totally tangential to the original issue — sorry! — but I'm trying to understand: in the useFragment implementation @jayy-lmao posted above, how does the hook react to cache updates? I can see that if the component happens to render again that it will get the new value for the fragment, but what triggers it to render when the cache is modified?

@jayy-lmao
Copy link

jayy-lmao commented Jun 11, 2021

@smikula you're totally right. I am a fool in a man's shoes.

We had some issues with it recently - turns out it only updates on 'client.writeQuery'.

I think I deceived myself when testing it. It was probably just the component using the hook re-mounting and running the hook again.
Even worse, useEffect didn't work with SSR for us and caused components to have to hydrate on the client side.

I did have a few attempts at forking apollo client library, and trying to create a useFragment that uses the Observables already within the client, but to no success. It looked like I would have also had to pass the query down to useFragment too for it to work :/

Happy to have another crack at a fork if anyone has any ideas of where I need to create a new observable/what I can listen to.

@smikula
Copy link

smikula commented Jun 11, 2021

@jayy-lmao Haha, no problem. Hopefully the client will soon (or eventually) support it natively and simplify life for us. I was going down the path of passing in the query, too, so it seems like that might be the best bet.

@vigie
Copy link
Author

vigie commented Jun 25, 2021

Hey @benjamn thanks for the happy news!

To make sure I understand the current plan correctly, it is to move local resolver functionality out of core and into an officially supported but separately installable link, is that correct?

If so that is a great outcome and I would be happy for this issue to be closed in favor of one tracking that feature.

I agree with you that the link stack always felt like a better home for this feature than core.

For example: I'm hoping that by moving it back to the link stack this will naturally solve one of my pet peeves with the current local resolver implementation, which is that, even if your local resolvers are asynchronous, the client never emits loading: true results, forcing components to simulate this locally.

On the useFragment front - I haven't fully grok'd this feature yet, but just wanted to note that I am an Angular user and I know there's quite a few of us. So hopefully react won't get exclusive access to some fundamental new feature that cannot be reflected in core.

Thanks again for engaging, it's very satisfying to find an OSS project that really listens and responds and it only increases my confidence in our investment in Apollo.

@fredericgermain
Copy link

Thank you for raising this issue.
We were about to consider a upgrade to v3... And the new local state of apollo v3 makes little sense to us.
typePolicies looks just to ugly to worth spending time on them.
We could see a point for reactive variables, as it follows the spirit of react hooks somehow...
We cannot figure out of to have defaults values initialized in the local store in v3 as easily as it was on v2.
Local resolvers were just so easy to go with. We have trouble understanding why it would be moved away.
Not possible to make the migration without massive refactoring that we don't want, we stay in v2, bearing a few react warnings.
Hopefully v4 brings a solution.

@jpvajda
Copy link
Contributor

jpvajda commented Aug 31, 2022

👋 I'm going to close this discussion out as we've captured this concern in #10060 which a new issue relating to #8189 and #8189 (comment)

Thanks for the dialog here it's much appreciated! 🙏

@jpvajda jpvajda closed this as completed Aug 31, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests