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

Expose selected fields in resolver context #5

Merged
merged 1 commit into from
May 10, 2018

Conversation

0xSalman
Copy link

@0xSalman 0xSalman commented May 8, 2018

This fixes graph-gophers#17. Basically, it exposes the selected field tree in the context for the resolvers and downstream usage.

There are two PRs open in upstream that fix this issue but I think the solution in this PR is better and we can merge it in our branch to make use of it quickly. The PRs in upstream have been open for a while.

Does this break the existing api?

No

What if I do not want the selection tree? Does it have performance impact?

There should be very negligible performance impact. The selection tree is lazily added; meaning, it is added to the context only when you request it. The lazy evaluation happens via a closure function.

How can I use it?

In your resolver, you can do selected.GetFieldsFromContext(ctx) which returns []selected.SelectedField, error. For an example, take a look at example/social/social.go

Test Notes

  • Run ./example/social/server/server.go
  • Visit http://localhost:9011/ and run this query. Verify that some basic stuff about selection tree is logged in the console
query GetUser($userId: ID!, $page: Pagination) {
  user(id: $userId) {
    id
    name
    email
    phone
    friends(page: $page) {
      id
      name
    	email
    	phone
    }
  }
}

variables:

{
  "userId": "0x02",
  "page": {
    "first": 1,
    "last": 0
  }
}

TODO

  • add unit tests

@0xSalman
Copy link
Author

0xSalman commented May 8, 2018

FYI, we are merging this PR to dt-master because we have a PR open against master in upstream and do not want these changes to appear there. I have made dt-master as the main branch for our fork to avoid conflicting with any open upstream PRs.

Copy link

@Niennienzz Niennienzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through the code. Verified sample server with sample query. Also tried queries with more nested levels, it worked nicely.

@SteveGBanton
Copy link

Reviewed code, tested by integrating into helix repo, works.

@abourget
Copy link

I'd like to have that upstream :) Lots of forks around!

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

Successfully merging this pull request may close these issues.

No way to get requested fields inside resolver
4 participants