Gracefully support Query.node in @exposeField; clearer behavior when missing #648
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
September 7, 2025 13:48
…n when Query.node is absent; add dev shell for local builds
…e when Node is implemented (refetch generation handled gracefully)
|
(generated with gpt5 pro) |
|
per patryk: here we need to add another condition, object_implements_node && Query.node is "valid" don't know if we can do it at this step |
rbalicki2
reviewed
Sep 13, 2025
|
|
||
| unprocessed_items.push(SelectionType::Scalar(unprocessed_scalar_item)); | ||
| match unvalidated_isograph_schema | ||
| .create_new_exposed_field(expose_as_field, parent_object_entity_name) |
There was a problem hiding this comment.
let's have create_new_exposed_field return a Result<Option<...>> and leave this code unchanged
| if field == "node" { | ||
| let Some((query_name, _)) = self.find_query() else { | ||
| return Err(WithLocation::new( | ||
| CreateAdditionalFieldsError::PrimaryDirectiveFieldNotFound { |
There was a problem hiding this comment.
and here, let's return None
| .expect("canonicalize cwd") | ||
| .to_string_lossy() | ||
| .to_string(); | ||
| db.set(CurrentWorkingDirectory(cwd_str.intern().into())); |
There was a problem hiding this comment.
we don't need to use the tmp folder, we can a bunch of calls to db.set(...) instead. Happy to figure out how to make that work unless you're interested in it. But not a blocker, the test lgtm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
field: "node.asType"but the schema lacksQuery.node.nodeas rooted at the Query type and checks for existence.Query.nodeis absent, skips generating the refetch field instead of erroring (and returns a more specific error variant internally).Motivation
Nodetriggers a refetch field via@exposeField(field: "node.as<Obj>"). In schemas withoutQuery.node, compilation fails with "Invalid fieldnodein @exposeField directive".Nodebut intentionally do not exposeQuery.node.Changes
Query.nodeis absent (clearer than a generic InvalidField).Query.nodeis missing; continue compiling the rest of the schema.Notes
Query.node, while making compilation resilient for projects that don’t.Query.nodewhenever any type implementsNode; this PR aims to be permissive and avoid breakages while improving the error.Testing
Query.node, the compiler proceeds without failing.Related gist/repro: https://gist.github.com/randallb/df13780a215c74f3f9e05d81f4fe6e03