apollographql / apollo-feature-requests

πŸ§‘β€πŸš€ Apollo Client Feature Requests | (no πŸ› please).
Other
130 stars 7 forks source link

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

Open martynaskadisa opened 3 years ago

martynaskadisa commented 3 years ago

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
benjamn commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

image

theguriev commented 3 years ago

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 commented 3 years ago

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

theguriev commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

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 commented 2 years ago

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.