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

Add fieldPath to missing data field error logs #4835

Closed
wants to merge 1 commit into from
Closed

Conversation

captbaritone
Copy link
Contributor

For @required fields and Relay Resolvers which might error, we include the field's path (relative to its parent fragment/query) in the generated artifact.

This allows us to avoid maintaining a field path during read traversal, which would require a large number of push/pops of an array as we traverse through the reader AST.

However, with the addition of @throwFieldOnError we also report missing data as field errors, but these errors can occur for any field, and including every field's path in the generated artifact is likely too heavy.

This PR attempt to use an efficient approach where we build up the path on the trailing edge of the reader recursion, which allows us to avoid pushing/popping in the common case where there are no errors. The cost in the happy path is just an additional method call and a few null checks as we exit each level of recursion that impacts the path.

If this approach proves effective, we should be able to adopt the same approach for @requried and resolver errors, simplifying the compiler and making our generated artifacts a bit lighter.

@facebook-github-bot
Copy link
Contributor

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@captbaritone merged this pull request in 3eb627d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants