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

Don't use Viewer #1

Closed
leebyron opened this issue Oct 21, 2016 · 9 comments · Fixed by #2
Closed

Don't use Viewer #1

leebyron opened this issue Oct 21, 2016 · 9 comments · Fixed by #2

Comments

@leebyron
Copy link

Love this!!!

Just a suggestion to not put everything on a Viewer type, which is an antipattern in GraphQL servers, but instead to put all root access on the Query type.

@lucasbento
Copy link
Owner

Hello @leebyron, I'm really glad that you took your time for opening an issue!

That's what I initially had in mind but that's the only workaround that I saw since I needed it to work with Relay 1, which doesn't seem to handle multiple fields on the Query type: facebook/relay#112.

@leebyron
Copy link
Author

We should probably go back and edit that workaround as its no longer what we would suggest doing. It's based on a strange internal Facebook-ism that were trying to get rid of. I'd hate for new non-Facebook projects to have to suffer from it.

A better workaround that's less invasive is a field on the query type that returns itself.

So:

type Query {
  query: Query
  # all your other top level fields here 
}

That lets people using your API largely ignore the workaround, but for the rare case you're using relay and accessing multiple fields you can just wrap them in query { }

@stubailo
Copy link

Yes please! I love this kind of workaround that works with Relay but still lets other people not have to deal with it.

@rturk
Copy link
Collaborator

rturk commented Oct 23, 2016

@leebyron before this issue I had no idea that viewer was no longer required for Relay, maybe this needs to be clarified/documented in Relay docs

@maletor
Copy link

maletor commented Nov 7, 2016

@leebyron, this is OK workaround but you still can't have

query { query { node(id: $id) } }

So you need to create another "route" in Relay. Really looking forward to Relay 2.

@leebyron
Copy link
Author

leebyron commented Nov 7, 2016

@maletor that query should work just fine with the workaround I suggested above.

But yes, Relay still requires both a top level field called node and requires one top level field per route. We're hoping to drop those requirements in the future, but this work around at least patches over the issue for most cases by allowing multiple fields on the nested "query" field

@maletor
Copy link

maletor commented Nov 8, 2016

Is this also related to why my connection type at the top level of my schema doesn't work with RANGE_ADD in Relay?

For instance, I'm trying to create a mutation that adds a node to a connection but that connection itself has no parent node -- it's just a query on the top level.

@sibelius
Copy link
Collaborator

sibelius commented Aug 24, 2017

@pcattori
Copy link

pcattori commented Mar 27, 2018

@leebyron I was tempted to use the viewer pattern to pass in an authentication token after reading this article (go to "GraphQL Mechanics" section for the relevant bit).

My aim was to centralize auth handling, rather than having each top-level query type have to accept a token parameter (which then each resolver would need to verify).

{
  viewer(token: "<some authentication token>") {
    myProfile
    mySettings
    myFriends
  }
}

AFAIK I can't pass an argument directly to the root Query type, but only to the fields. Any tips on how to tackle that use-case w/o viewer? Or would you consider the viewer pattern acceptable in that context?

(I guess I could use the request header context with some express middleware, but was trying to handle everything in GraphQL-layer with resolvers)


Nevermind! I see that I can still use external tools to set authentication headers for GraphiQL and then read request headers via context.

In my case, I'm going to use apollo-resolvers to centralize auth logic.

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 a pull request may close this issue.

7 participants