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

read method on field's TypePolicy should allow async behaviour #383

Open
martynaskadisa opened this issue Aug 17, 2020 · 14 comments
Open

read method on field's TypePolicy should allow async behaviour #383

martynaskadisa opened this issue Aug 17, 2020 · 14 comments
Labels
📕 cache Feature requests related to the cache

Comments

@martynaskadisa
Copy link

Intended outcome:

I was trying to add a read method on a field. Inside this method I'd perform an async operation and return a promise with the data (see codesandbox below).

This was my effort to upgrade existing AC2 app to AC3 and migrate away from local resolvers.

Actual outcome:

My expected outcome was that Apollo Client would wait for that promise to resolve and only then would render data in my component. However, it accepted the returned promise, did not wait for it to settle and attached it under data in useQuery hook.

This makes migrating away from local resolvers much much more difficult since local resolvers can wait for data to resolve before returning response where read from Type Policies cannot.

How to reproduce the issue:

See this codesandbox's index.js and App.js files
https://codesandbox.io/s/apollo-type-policy-async-read-3d1fj?file=/src/index.js

Versions

  System:
    OS: Windows 10 10.0.19041
  Binaries:
    Node: 14.0.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.4 - C:\dev\project\node_modules\.bin\yarn.CMD
    npm: 6.14.4 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 84.0.4147.125
    Edge: Spartan (44.19041.423.0), Chromium (84.0.522.59)
  npmPackages:
    @apollo/client: 3.1.3 => 3.1.3
    apollo: 2.30.2 => 2.30.2
    apollo-upload-client: 14.1.1 => 14.1.1
@martynaskadisa martynaskadisa changed the title read method on TypePolicy should allow async behaviour read method on field's TypePolicy should allow async behaviour Aug 17, 2020
@benjamn
Copy link
Member

benjamn commented Aug 17, 2020

At a superficial level, async read functions are not possible because the cache.read{Query,Fragment} methods are synchronous, so everything involved in reading a query from the cache must also be synchronous. Reading from the cache needs to be synchronous because many UI frameworks (including React, at least without Suspense) have synchronous rendering pipelines, so it's important for UI components to have immediate access to any existing data.

However, you can implement a read function that is effectively asynchronous using reactive variables and the options.storage object:

new InMemoryCache({
  typePolicies: {
    Person: {
      fields: {
        asyncName: {
          read(existing, { storage }) {
            if (!storage.var) {
              storage.var = makeVar("unknown");
              delay(100).then(() => storage.var("Foo"));
            }
            return storage.var();
          }
        }
      }
    }
  }
})

Although this may seem a bit convoluted compared to an async function, it has a fairly significant advantage: once options.storage.var has been updated, all future reads can return the correct value immediately, whereas an async function would need to be called repeatedly, and would return a new Promise every time.

To make this pattern simpler, we believe we may be able to provide a helper function that would work something like this:

new InMemoryCache({
  typePolicies: {
    Person: {
      fields: {
        asyncName: {
          read: asyncRead(async (existing, options) => {
            await delay(100);
            return "Foo";
          }),
        }
      }
    }
  }
})

The asyncRead helper function (to be implemented) would return a read function that behaves essentially like the makeVar/storage code I showed above. If you're feeling industrious/impatient, you might try writing it yourself. Happy to answer questions here if you get stuck.

@martynaskadisa
Copy link
Author

Hey @benjamn, thanks for your response! I think I finally get why read calls have to be sync in Apollo Client.

The asyncRead helper's API looks kinda neat. However, if I understand your code correctly there will be a certain time frame where components trying to read this field via useQuery will get undefined which could be unwanted and quite confusing.

Example:

const client = new ApolloClient({
  client: new InMemoryCache({
    typePolicies: {
      Person: {
        fields: {
          asyncName: {
            read: asyncRead(async (existing, options) => {
              await delay(100);
              return "Foo";
            }),
          },
        },
      },
    },
  }),
});

// Somewhere in a React tree under ApolloProvider
const Component = () => {
  const { data, loading, error } = useQuery(gql`
    query ComponentQuery {
      people {
        name,
        asyncName @client
      }
    }
  `)

  if (loading) {
    return <Loading />
  }

  return (
    <div>
      {data.name} {/** <- this will be defined */}
      {data.asyncName} {/** <- this still might be `undefined` right after `loading` flips from `true` to `false` */}
    </div>
  );
}

What I'd imagine that helper would let me do is somehow teach the client to wait for all data (remote and local) to be resolved and only then render, but unfortunately this seems not to be the case.

I can't help but wonder, perhaps my use case is a bit niche and maybe it's better off handled in a custom link where I could extend schema in a similar fashion as in v2. But for the time being I think I'll be sticking to local resolvers for now 😢

@kirlat
Copy link

kirlat commented Aug 28, 2020

What I'd imagine that helper would let me do is somehow teach the client to wait for all data (remote and local) to be resolved and only then render, but unfortunately this seems not to be the case.

I'm in a similar situation myself and the best solution I've came up so far with is, instead of asyncName returning either a string or an undefined make it return an object with the loading status as below:

{
  name, // A string
  loading // A boolean
}

When the loading would flip from true to false the client will know that the final value is delivered to the name filed. So it's like a loading status of a GraphQL query, but on a field level. It's somewhat cumbersome for my taste but better than nothing. At least it allows to understand when the results are delivered exactly.

@theguriev
Copy link

At a superficial level, async read functions are not possible because the cache.read{Query,Fragment} methods are synchronous, so everything involved in reading a query from the cache must also be synchronous. Reading from the cache needs to be synchronous because many UI frameworks (including React, at least without Suspense) have synchronous rendering pipelines, so it's important for UI components to have immediate access to any existing data.

However, you can implement a read function that is effectively asynchronous using reactive variables and the options.storage object:

new InMemoryCache({
  typePolicies: {
    Person: {
      fields: {
        asyncName: {
          read(existing, { storage }) {
            if (!storage.var) {
              storage.var = makeVar("unknown");
              delay(100).then(() => storage.var("Foo"));
            }
            return storage.var();
          }
        }
      }
    }
  }
})

Although this may seem a bit convoluted compared to an async function, it has a fairly significant advantage: once options.storage.var has been updated, all future reads can return the correct value immediately, whereas an async function would need to be called repeatedly, and would return a new Promise every time.

To make this pattern simpler, we believe we may be able to provide a helper function that would work something like this:

new InMemoryCache({
  typePolicies: {
    Person: {
      fields: {
        asyncName: {
          read: asyncRead(async (existing, options) => {
            await delay(100);
            return "Foo";
          }),
        }
      }
    }
  }
})

The asyncRead helper function (to be implemented) would return a read function that behaves essentially like the makeVar/storage code I showed above. If you're feeling industrious/impatient, you might try writing it yourself. Happy to answer questions here if you get stuck.

Hey @benjamn
I tried your approach but there is another problem... Why the data which was received asynchronously will not record into cache?

As u can see my cache is empty after all these manipulations...
image
image
image

@theguriev
Copy link

image

@theguriev
Copy link

Should I do it manualy? Right?
Like this:

if (!storage.var) {
            storage.var = makeVar(null);
            get(`performer/performers?${urlQuery}`)
                .then(
                    res => res.map(
                        el => ({
                            ...el,
                            id: el.performerId,
                            __typename: 'performers'
                        })
                    )
                )
                .then(
                    data => {
                        storage.var(data)
                        cache.writeQuery({
                            query: getPerformers,
                            data: {performers: data}
                        })
                    }
                )
        }
        return storage.var();

@theguriev
Copy link

@benjamn And how I can set the loading state to true in this case? It's false always for me.

@theguriev
Copy link

Nevermind. I figured out. Instead of makeVar(null) u should use makeVar(undefined). Here is an example of asyncRead function

import { makeVar } from '@apollo/client'

export const asyncRead = (fn, query) => {
    return (_, args) => {
        if (!args.storage.var) {
            args.storage.var = makeVar(undefined)
            fn(_, args).then(
                data => {
                    args.storage.var(data)
                    args.cache.writeQuery({
                        query,
                        data: { [args.fieldName]: data }
                    })
                }
            )
        }
        return args.storage.var()
    }
}

and then loading state will detect properly.

@ManAnRuck
Copy link

ManAnRuck commented Sep 23, 2020

i am unable to get this to work 😢
i want to read data from my asyncStorage.
when I console log within the typePolicies object, I get the correct output
within my component where I want to use it I get always undefined

maybe a clear documentation or prepared helper function could help

@MarMun
Copy link

MarMun commented Sep 24, 2020

@gcofficial I have an async problem with a query field and I'm wondering if your asyncRead could help.

Would be awesome if you could tell me how I would wrap this in your proposed asyncRead function:

  Query: {
    fields: {
      async userTest (_, { variables, toReference }) {
        const userTest = await datasource.get(variables.id)
        return toReference({
          __typename: 'UserTest',
          ...userTest
        }, true)
      }
    }
  }

@theguriev
Copy link

theguriev commented Sep 25, 2020

@gcofficial I have an async problem with a query field and I'm wondering if your asyncRead could help.

Would be awesome if you could tell me how I would wrap this in your proposed asyncRead function:

  Query: {
    fields: {
      async userTest (_, { variables, toReference }) {
        const userTest = await datasource.get(variables.id)
        return toReference({
          __typename: 'UserTest',
          ...userTest
        }, true)
      }
    }
  }

Hey 👋🏻 @MarMun

It will be something like this...

Query: {
    fields: {
      userTest: asyncRead(() => {}, query)
    }
  }

@stretch0
Copy link

stretch0 commented Mar 5, 2021

Is there a way around this for the mean time?

Rather than trying to read from async storage within the cache, is it possible to use something like apollo-cache-persist so that the whole cache just gets saved to AsyncStorage and persists over app reloads?

@wallzero
Copy link

wallzero commented Mar 23, 2021

I attempted to migrate some resolvers over to type policy read and merge functions. My thoughts:

  • FieldFunctionOptions need to include the name of the Query or Mutation that caused the read or merge. Particularly with Mutations (but occasionally with reads as well) the behavior of interacting with local state can change and resolvers can handle this. EDIT: I can use Mutation field policies
  • read and merge properties need to support async functions or Apollo should provide a wrapper like the example above.
  • read and merge functions need type safe examples (including async examples).
  • Migration documentation will be needed in advance before resolvers can be removed.

For now I've reverted my changes and will have to stick with resolvers. Although, I've seen how the flexibility of local resolvers can be abused and agree restrictive read and merge type policies have merit.

@Michael-M-Judd
Copy link

Michael-M-Judd commented Jan 10, 2022

Commenting here in case anybody else needs this. I recently found that returning a reactive var with undefined no longer results in useQuery loading while it resolves to a value. Additionally, returning the reactive var no longer updated when the value was updated from the cache. To fix this, we did something that wasn't ideal but it worked:

import { ReadFieldFunction } from '@apollo/client/cache/core/types/common';

// this makes the cache from a field-read
const cacheId = (readField: ReadFieldFunction): string => `${readField('__typename')}:${readField('id')}`

// in your type policies
const typePolicies = {
  Query: {
    fields: {
      SomeType: {
        fields: {
          yourAsyncField: {
            read(existing, { readField, cache }) {
              doSomethingAsync().then((result) => {
                cache.modify({
                  id: cacheId(readField),
                  fields: {
                    yourAsyncField: result,
                  },
                });
              });

              return existing;
            },
          },
        },
      },
    },
  },
};

We modify the cache directly on the object (assuming your field has an id and a typename). This does mean that you can get a flash of content before this happens, but we've handled that by temporarily returning the value as loading.

@alessbell alessbell added the project-apollo-client (legacy) LEGACY TAG DO NOT USE label Apr 28, 2023
@alessbell alessbell transferred this issue from apollographql/apollo-client Apr 28, 2023
@jerelmiller jerelmiller added 📕 cache Feature requests related to the cache and removed project-apollo-client (legacy) LEGACY TAG DO NOT USE labels Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📕 cache Feature requests related to the cache
Projects
None yet
Development

No branches or pull requests