Field return issue depending on field order in same query

Banou26 commented 10 months ago

Describe the bug

Depending on if a field is before or after another field, its value is null or defined using updates & resolvers.

e.g

media {
  uri
}
medias @stream {
  uri
}

will return

"media": null,
"medias": [...]

while

medias @stream {
  uri
}
media {
  uri
}

returns

"medias": [...]
"media": {
  uri: ...
}

I'm not sure if that's normal behavior with my resolvers, but I'm reporting just in case it's a fixable issue / to document it for anyone else also hitting that edge case.

Reproduction

https://github.com/Banou26/urql-issue-3385/blob/main/src/main.tsx#L106-L138

Urql version

urql 4.0.5

Validations

[X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
[X] Read the docs.
[X] Follow our Code of Conduct

JoviDeCroock commented 10 months ago

That is actually expected, in graphcache we execute the resolvers/directives in the order they are encountered within the document so it could be that i.e. the next field hasn't resolved yet which would make it undefined. I guess we could make resolve all the fields without user-defined logic first but that's probably quite a big byte and performance hit to take.

kitten commented 10 months ago

As per the above, just because you can access arbitrary data from a result (be that an API result or a cache result that's being built up) that doesn't mean that that's a great idea 😅

There are several issues in your config that are preventing me from being able to help you, and to put it simply, it's too complex.

The short answer here is that some fields will always be unavailable. If you have fields a and b that are read from the cache and you have resolvers for both that are accessing each other, one will always be unavailable when the other is resolved. In other words, you must not depend on other properties being available on the parent object in most cases.

There are several things that, personally, I wouldn't recommend in your code sample, and most may come down to schema design and trying to write a configuration that's just a little too "out there" and complicated but I'll list a couple points here to summarise what I mean.

Using non-scalar values in keys. I understand that you're trying to derive keys in your configuration to have relational joined keys for your edges based on their children, but I'm not convinced that's a good idea. Episode for instance could have its own id and from a glance, the key being predictable doesn't really help you that much probably
Overriding a result value as in updates.Episode isn't a supported pattern. so any behaviour that you'll see there is unsupported. This also shouldn't ever be necessary
Deriving a field from a key, such as in your resolvers, is not recommended. While it's possible to use keys as simple strings, they're basically meant as opaque values. This isn't enforced but that's basically like deriving a value from an ID

It basically looks a little like you're trying to normalise data on the client-side that isn't normalised on the schema. Furthermore, I wouldn't recommend to try to normalise edges. You could give them a normalised ID on your schema (after which it would also make more sense to teach Graphcache what that ID looks like if it's predictable) but that also depends on whether you really need to be able to access an edge independently of its parent, which, again, hinges on how you access these edges and how they "re-occur" in your schema.

Say, if you have a details view and a list view. If that's the only way to get from a list of entities to a single entity, and if you then have the edge entity on a separate field or fields, as long as that entity has an ID field, there isn't any need to derive a predictable key in Graphcache.

I'll close this for now, but feel free to post more questions here ✌️

Banou26 commented 10 months ago

Thanks for the descriptive answer!

The truth is that I'm hacking together yoga & urql into being a graph database & graphql client for my project. I'm a very edge case user and I'm definitely not using urql in the conventional way. I used Apollo Server/Client with good success but it had terrible performance issues so I'm currently moving to urql because of that. Using @stream simplified my logic by a thousand times, even if it's dependent on undefined cache resolver behaviors.

All of my data doesn't come from an actual graphql API server with normalized data that i host/control but instead, run in yoga from the browser, that executes scrapers that returns handles that might link to the same "end resource" or not.

I need to regroup these related resources into one accessible and queryable element for my frontend, that's what I'm doing through the "scannarr" elements here. This was the simplest way for me to do that, that i found, to group related resources into a simple renderable element. (this repro doesn't have a lot of the actually more complex field grouping logic)

Overriding a result value as in updates.Episode isn't a supported pattern. so any behaviour that you'll see there is unsupported. This also shouldn't ever be necessary

I've actually tried not modifying the actual result in the updates resolver, using cache.link, cache.writeFragment, ect... but I've somehow never managed to actually modify the cache values. So i ended up trying this and it worked perfectly for my case. I'd prefer not having to do this TBH but I've lost too much time trying the "documented way".

If you're curious about the project, It's https://github.com/Banou26/scannarr (very much WIP right now since i've broken everything to debug this issue lol, but it should be back to normal in a few hours/days), that I'm using in https://github.com/Banou26/stub

urql-graphql / urql