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

(graphcache) - cache.resolve(parent, ...) doesn't seem to work #1207

Closed
nghiepdev opened this issue Dec 8, 2020 · 5 comments · Fixed by #1208 or #1219
Closed

(graphcache) - cache.resolve(parent, ...) doesn't seem to work #1207

nghiepdev opened this issue Dec 8, 2020 · 5 comments · Fixed by #1208 or #1219
Labels
bug 🐛 Oh no! A bug or unintented behaviour.

Comments

@nghiepdev
Copy link

Hi guys,

After upgrade to @urql/[email protected] cache.resolve doesn't seem to work always return null value.

export const cache = cacheExchange({
  resolvers: {
    City: {
      name(parent, args, cache) {
        const id = cache.resolve(parent, "id");
        const name = cache.resolve(parent, "name");

        console.log({ id, name }); // null , null

        return `ID: ${id} - ` + name;
      }
    }
  }
});

urql: 1.11.4
@urql/exchange-graphcache: 3.3.1

Steps to reproduce
Check log tab
https://codesandbox.io/s/headless-wildflower-ynjsk

The repo older version still work
https://codesandbox.io/s/prod-monad-gmvyk

@nghiepdev nghiepdev added the bug 🐛 Oh no! A bug or unintented behaviour. label Dec 8, 2020
@kitten kitten added documentation 📖 This needs to be documented but won't need code changes and removed bug 🐛 Oh no! A bug or unintented behaviour. labels Dec 8, 2020
@kitten
Copy link
Member

kitten commented Dec 8, 2020

This probably needs a change in the documentation, but this is still working as intended. What's happened is that basically __typename won't be added to entities anymore automatically by Graphcache but in-order, like for a normal field, only if it's defined. Obviously since we add __typename fields to query documents, it'll always be (in some form) defined for each selection set. But it may be added later on. #1185

I believe the documentation sometimes speaks about using cache.resolve(parent, ...), however afaik that has never been safe. This isn't safe because the parent may not have its id field (or any other keyable field) added to it by the time the resolver with cache.resolve is reached.

The safe method here is to use info.parentKey, e.g. cache.resolve(info.parentKey, 'name').

@nghiepdev
Copy link
Author

Thanks, @kitten I will change to info.parentKey instead.

@kitten kitten added bug 🐛 Oh no! A bug or unintented behaviour. and removed documentation 📖 This needs to be documented but won't need code changes labels Dec 9, 2020
@kitten kitten changed the title @urql/[email protected] cache.resolve doesn't seem to work (graphcache) - cache.resolve(parent, ...) doesn't seem to work Dec 9, 2020
@kitten
Copy link
Member

kitten commented Dec 9, 2020

I've discussed this with @JoviDeCroock and we came to the conclusion that this actually should be supported. We did use it ourselves in the documentation and it's a pretty intuitive way to resolve fields gradually from a parent, instead of using info.parentKey, so I'm opening a patch that automatically aliases parent to info.parentKey in all cases.

@nghiepdev
Copy link
Author

@kitten
Copy link
Member

kitten commented Dec 10, 2020

@nghiepit Nice catch! I believe I've actually forgotten to enable the additional case we were after. Follow up PR has been filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Oh no! A bug or unintented behaviour.
Projects
None yet
2 participants