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

[v2] Remove Viewer type. Relay Modern no longer needs this proxy. #1906

Merged
merged 1 commit into from
Aug 27, 2019

Conversation

alloy
Copy link
Contributor

@alloy alloy commented Aug 27, 2019

Part of https://artsyproduct.atlassian.net/browse/PLATFORM-1718
Emission PR: artsy/emission#1818

Relay Classic had a few constraints on root fields, which are described in facebook/relay#112. A common workaround, which originated with FB, was to have a root field that would proxy all the root fields. This was often done on a field called viewer.

Besides Relay Modern no longer having these constraints, I also think that we should reserve the name viewer for any potential future case where we want to represent e.g. the user looking at the site/app, perhaps aside from me.

@zephraph
Copy link
Contributor

Can you add some more context? I was trying to follow along w/ the graphql slack conversation, but I don't fully understand this or why it was needed to begin with.

@alloy
Copy link
Contributor Author

alloy commented Aug 27, 2019

Done. Let me know if that helps.

@zephraph
Copy link
Contributor

Is this blocked waiting on the Emission PR?

@alloy
Copy link
Contributor Author

alloy commented Aug 27, 2019

@zephraph Yup, but that’s up now too, so this is ready to review/merge 👍

@zephraph zephraph merged commit ac33d57 into master Aug 27, 2019
@zephraph zephraph deleted the v2-remove-viewer branch August 27, 2019 20:45
@artsy-peril artsy-peril bot mentioned this pull request Aug 27, 2019
@damassi damassi mentioned this pull request Sep 25, 2019
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