facebook
Add context to relay resolvers
This is a draft PR to implement an approach mentioned in #4000.
I've currently put the resolverContext on the Store instead of the Environment. It seems that the Store might be a better place than the Environment?
If defined on the environment the context would need to go from the environment into the store down to the reader? Since a store could be used by different environments if the resolverContext would be on the environment that might cause more issues than it being on the store?
The following is an example of how this would be used.
One would provide the context as follows.
const store = new LiveResolverStore(source, {
resolverContext: {
counter: createObservableCounter(),
world: "World!"
},
});
Then the resolvers could use the context similar to readFragment
/**
* @RelayResolver Query.hello_context: String
*
* Say `Hello, ${world}!`
*/
import {resolverContext} from '../../ResolverFragments';
function hello_context(): string {
const world = resolverContext<{world: string}>().world;
return `Hello, ${world}!`;
}
/**
* @RelayResolver Query.hello_world_with_context: String
* @live
*
* Say `Hello ${world}!`
*/
function hello_world_with_context(): LiveState<string> {
const dependency = resolverContext<{greeting: string}>();
return {
read() {
return `Hello ${dependency.greeting}!`;
},
subscribe(callback) {
return () => {
// no-op
};
},
};
}
Hi @Markionium!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
Hey @Markionium thanks for picking this up!
I agree that attaching the context to the store is the right choice.
However, rather than accessing the context via a magic function, my preference would be that the context object get passed in as the third argument to the resolver function. This would keep alignment with a traditional graphql-js style resolver function.
/**
* @RelayResolver User.bestFriend: User
*/
export function myResolver(user, _, ctx) {
return ctx.userById(user.bestFriendId);
}
In terms of type safety, I think we can allow users to provide a module/export name in the compiler config for the project. We can then include that type in our generated type assertions (note that these are not yet built for TypeScript, but it should be pretty easy to add if you are interested in another project).
You still need to ensure your compiler config and the store you create actually match, but that's something you can get right once and in one place so probably an okay end state.
Beyond that we are also exploring a more light-weight way to define resolvers using an approach similar to Grats. In that world it could be even simpler, and work similar to how it works in Grats (see the docs here).
Connecting the dots. @alloy and I have been discussing this a bit more on this Gist: https://gist.github.com/alloy/f0c9c90ff7a28f3b17850021488979d5
Looks like this is getting close, but still in draft. Give me a ping when this is ready for review! Excited to see this!
Hey @captbaritone! We're ready for your review here. @Markionium can you please un-mark as draft?
Can we also add a docs page in this directory that describes how to use context? https://github.com/facebook/relay/tree/main/website/docs/guides/relay-resolvers
The args page is probably a good starting place.
@captbaritone @Markionium @alloy Ready for another look here. Addressed all comments and added some fixture tests as well. Feel free to suggest any additional tests you'd like to see.
All rust tests are passing on my local, seems to be failing on ubuntu but not mac in CI.
Rust test failures on CI are unrelated. We have an ongoing issue where it is running out of disk space.
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
When trying to land this internally I found that it was going to be very difficult to rollout this change safely if we change how resolvers that don't read a parent object or rootFragment get called. So, I've updated the diff a bit internally to go back to the previous behavior where the first argument is omitted if there is no rootFragment and the resolver is not on a model type. I've updated the docs to match.
@captbaritone merged this pull request in facebook/relay@2973a0fca595d6a767eeaaba457ce8d09c173a0d.