Skip to content

Expose selected fields in resolver context#5

Merged
0xSalman merged 1 commit into
dt-masterfrom
expose-selection-tree
May 10, 2018
Merged

Expose selected fields in resolver context#5
0xSalman merged 1 commit into
dt-masterfrom
expose-selection-tree

Conversation

@0xSalman

@0xSalman 0xSalman commented May 8, 2018

Copy link
Copy Markdown

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

0xSalman commented May 8, 2018

Copy link
Copy Markdown
Author

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.

@Niennienzz Niennienzz left a comment

Copy link
Copy Markdown

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
Copy Markdown

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

@abourget

Copy link
Copy Markdown

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