Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Remove ViewerType pattern from Relay Modern example #14

Closed
robrobkay51 opened this issue Feb 9, 2017 · 7 comments
Closed

Remove ViewerType pattern from Relay Modern example #14

robrobkay51 opened this issue Feb 9, 2017 · 7 comments

Comments

@robrobkay51
Copy link

robrobkay51 commented Feb 9, 2017

I recently came across this comment by Lee Byron which advises against using the Viewer type pattern in GraphQL/Relay applications: lucasbento/graphql-pokemon#1 (comment)

If this is now considered best practice, it would be helpful if all examples within this Repo were updated to reflect the guidelines outlined in the above issue thread.

@wincent
Copy link
Contributor

wincent commented Feb 12, 2017

Thanks for the suggestion, but I don't think we should consider doing this until the new Relay core is out (which has much less hacky support for the Viewer type, and drops existing restrictions around root fields). While it's true that Viewer is somewhat of a Facebook-ism, that doesn't necessarily make it an anti-pattern (I think Lee's language is a bit too strong there); it's a convention that has served us well, is perhaps a bit quirky, and one which perhaps we wouldn't have encouraged if we could go back a few years and design the system from scratch. For a related comment I just made on this, see here.

Having said all that, I am totally happy to remove Viewer from the examples. But let's wait until the new core and APIs are fully rolled out.

@ardalann
Copy link

@wincent @leebyron Now that Relay Modern is out, is there an example of this?
The TodoModern example still uses the Viewer field:
https://github.com/relayjs/relay-examples/blob/master/todo-modern/data/schema.js

@wincent
Copy link
Contributor

wincent commented May 24, 2017

It would be fine to remove the Viewer type from the modern example if you wanted to. There is no "official" example of making that kind of change yet (if there was, it would be here); for an unofficial example, here's the commit in my blog repo where I got rid of the User and Viewer types once I'd moved to Relay Modern.

Adding a "help wanted" label in case anybody wants to take a stab at this.

@wincent wincent changed the title Remove ViewerType pattern from all examples. Remove ViewerType pattern from Relay Modern example May 24, 2017
@fernandocalsa
Copy link

Is this issue still relevant? I'm starting with Relay Modern and I would like to help on this issue, I think it can help me to understand better the framework. I've never worked with GraphQL before so I don't know about Relay Classic. @wincent do you think a beginner can fix this issue?

@fernandocalsa
Copy link

thank you @sibelius I will try to work on it whenever I have some time :D

@Stephen2
Copy link
Contributor

Stephen2 commented Feb 9, 2019

#88 closes this

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

No branches or pull requests

7 participants