Provided variables and Live Resolver modules should allow injected dependencies

[leoasis]

Looking at how Provided variables modules should be defined, and how Live Resolvers modules are defined, the way it is currently expected to access external services (like the Redux store, or a module to ask for feature flags), is by using singletons at the module level.

This is not compatible with Node.js environments (when doing SSR for example), where the modules are loaded once and multiple requests are handled sharing the same module state.

It would be ideal if the signature for provided variables modules and live resolvers allowed to get access to a "context" object that could be injected in the Relay Environment, so that we could put these services to be accessed by the modules, instead of accessing them as singletons. Something like this:

const store = ...
const featureFlags = ...
const environment = new Environment({
  moduleContext: { store, featureFlags } // name TBD
});

return <ReduxProvider store={store}>
  <RelayEnvironment environment={environment}>
    <App />
  </RelayEnvironment>
</ReduxProvider>

And they would be consumed like this:

// a Provided Variable
export default {
  get(moduleContext): boolean {
    return moduleContext.featureFlags.check('todo_should_include_timestamp');
  },
};

// a Live Resolver
export default function RootAllTodosResolver(moduleContext) {
  return selectLiveState(moduleContext.store, (state) => {
    return state.todos.map((todo) => todo.id);
  });
}

Would this be something that would be acceptable for contribution? Thank you in advance for reading this!

[captbaritone]

Thanks for writing this up! Overall it looks right to me. Some concerns to track:

1. We need to convey the fact that these context values are not expected to change overtime. There will not be a mechanism to have dynamic values in this context (unless you are tracking them yourself like with a live resolver).
2. There's not really any way for these values to be typesafe. We can't statically know which environment a given resolver/variable provider will be used with. This is the same limitation that React Context has, so I think it's acceptable, but it's worth considering. How will we type these context values?
3. How will we thread this context value though?

I think a good next step would be a lightweight proposal for how we can achieve #3. This could be a rough PR, or even a document.

[leoasis]

Thank you Jordan for the quick reply, that makes sense, I'll try to work on a proposal for threading the value to the right places.

For 1, agree, this is meant for passing down static dependencies. Would freezing the context in development be enough to convey this? We could enforce that this context is a frozen object where properties cannot be modified.

Good point about type safety in 2. I agree there's no good way to enforce the context value will be of the defined type, but I think this is ok. Since you brought React Context as an example, maybe with an API similar to that, that would allow us to assert the context is of a specified type? Something like:

export default function someResolver(moduleContext: RelayModuleContext) {
  const store = readModuleContext(moduleContext, MyAppContext).store;
  // ...
}

Where readModuleContext would take an instance of a RelayModuleContext and a class and assert the context is an instance of it, otherwise fail. Or just an invariant could do as well.

If we go the readModuleContext approach, maybe we could even avoid having to receive the moduleContext from params, and instead do something similar to what readFragment does inside resolvers to get access to the current executing context (kind of like hooks).

But again, maybe this is too much, I think it's acceptable to allow any signature type in resolvers and provided variable modules, and since this would mostly be fixed per application, we could use other tooling like app-specific linters to enforce the right type is used.

[Markionium]

Hiya @captbaritone and @leoasis,

I took an attempt at implementing the approach discussed above in this draft #4704.

Here I've added the context to the store, as that seemed a more logical place than the environment? I might not be understanding this correctly however. :)

I think for the scenario where I'd like to use this approach having them as part of the store would be sufficient.

It seems adding them to the environment could possibly lead to a scenario where a store is used between environments with different contexts? (Which I think would be undesirable?)

There's not really any way for these values to be typesafe. We can't statically know which environment a given resolver/variable provider will be used with. This is the same limitation that React Context has, so I think it's acceptable, but it's worth considering. How will we type these context values?

I think this is still an issue. The consumer could type the context by wrapping it with function to enforce the typing.

Something like this, but this might also also not be ideal.

import {resolverContext as relayResolverContext, LiveResolverStore} from 'relay-runtime';

type ResolverContext = {
  greeting: string;
  counter: Observable<number>
}

export function resolverContext(): ResolverContext {
  return relayResolverContext<ResolverContext>();
}

export function createStore(resolverContext: ResolverContext) {
  return new LiveResolverStore(source, {
    resolverContext
  });
 }