Populate a normalized info.path instead of throwing#4
Conversation
|
So this is for the GraphQL-JS compatibility mode, right? To this I’d ask the question: do you want the real path for full compatibility? Because it’s possible, you just need to build indices and then you can path all objects across subtrees, have Claude port this: https://github.com/gmac/graphql-breadth-exec/blob/main/lib/graphql/breadth/executor/path_formatter.rb The downside of real object paths is that they’re expensive to compute (there’s a one-time up-front indexing cost, or else the indexing would have to get built into the main breadth loop and slow that down across all requests, which seems like a generally bad trade-off). But, it gives you the real “spec” data … which you then have to normalize back into a stable key, which is silly. I agree the stable key is generally more useful and it’s dirt cheap because it’s static. So, weird one: fast and practical, or slow and precise. My suggestion would be to add both. Hook the slow and precise path into its appropriate spot, then add a custom “schemaPath” accessor to info that provides the fast and static one. Then the original API is preserved, and the more useful cheap one is also available. thoughts? |
Interpreted field resolvers previously threw an ImplementationError on any `info.path` access, because breadth-first execution resolves every object at a level together and there is no per-object path with list indices. However, the most common reason resolvers read `info.path` is to build a stable per-field key (e.g. an ephemeral DataLoader cache key), for which the *normalized* path — the field's response-key ancestry without list indices — is exactly right, and is fully known from the ExecutionField scope chain. This populates `info.path` with that normalized Path (each segment carrying the typename it is selected on, matching graphql-js' addPath semantics), built lazily so resolvers that never read it pay nothing. Updates the interpreter test to assert the reconstructed path/typename chain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The interpreter previously populated info.path with a normalized path (the
field's response-key ancestry, no list indices), since breadth-first execution
resolves every object at a level together and there is no per-object path to
read off directly.
Per upstream feedback, offer both the spec-precise path and a cheap static one:
- info.path is now the real graphql-js Path for the object being resolved —
response keys plus real list indices — reconstructed on demand by a new
PathFormatter (ported from graphql-breadth-exec's Ruby Executor::PathFormatter).
Computing it indexes the scopes it walks, so it is built lazily and cached
per object index; resolvers that never read info.path pay nothing.
- info.schemaPath (a BreadthResolveInfo extension) carries the "fast and
static" path: the field's schema-name ancestry with no aliases or indices.
A dirt-cheap, stable key — what most path-keyed helpers (e.g. per-field
DataLoader cache keys) actually want.
The port also closes a gap the Ruby formatter never exercised (it is
instantiated but never wired in): abstract-type scopes bucket objects by
concrete type, so the parent field's result order does not line up 1:1 with a
concrete scope's objects. Indexing distributes the result-order suffixes back to
each concrete sibling by object identity, so every bucket keeps its real list
indices.
New tests assert info.path byte-for-byte against graphql-js's own info.path as
ground truth across nested lists and interleaved abstract-type buckets, plus
null-in-list index preservation and the static schemaPath shape.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
danielrearden
force-pushed
the
info-path-fix
branch
from
4570c57 to
e799233
Compare
|
Yeah, that makes sense to me. Made the suggested changes. |
2 participants
Right now interpreted resolvers throw if they touch
info.path, which makes sense given breadth execution has no per-object path. But in practice most resolvers that readinfo.pathjust want a stable per-field key (e.g. a DataLoader cache key) -- and the normalized path covers that and is fully available from the scope chain.This builds that normalized path lazily instead of throwing -- each segment carries its typename to match graphql-js
addPathsemantics, soinfo.path.prev?.typenamechecks still work. Resolvers that never read it pay nothing.Swapped the old "throws" test for one checking the reconstructed key/typename chain.
Happy to make it opt-in via an interpretSchema option instead if you'd rather not change the default.