Updates to context are not propagated to all operational providers
fridaystreet opened this issue · 3 comments
When using operational scope, updates to the context via middleware or schema directives is not passed to all providers in all modules. This becomes apparent for example when you have a directive on a query who's resolver is a provider in one module but the response type has child filed resolvers on another module
Say you have something like
Task module
Query {
myTasks: Tasks! @auth
}
type Task {
id: ID
notifications: [Notification]
}
(sudo resolver)
myQuery -> MyTaskProvider-> myTasks
Notifications Module
type Notification {
id: ID
message: String
}
(sudo resolver)
Notification -> NotificationProvider-> getNotification
When you query myTasks, the @auth directive runs and adds the user to the context. The task provider uses the user in the context no problem, but once it's handed to the the notification provider, by that point the Notification provider has already been instatiated for the operation and when getModuleContext is called, it's context is populated from the cache which was created when all the operational providers where instantiated, which doesn't include the updates that were applied from the @auth directive. This is just one example, there are other scenarios where the updated context is then not available, for instance using another module which is a service that isn't actually directly involved in the resolver chain.
Seems to be a couple of things going on from what we can see and just wanted to get some clarity on what The getModuleContext function is trying to achieve.
Firstly as described above it pulls the context from the cached context all the time and never checks to see if the context has been updated.
Secondly, when it doesn set the context, it only sets the context for the operational providers of the currently requested module.
The current function is here
Below is our current workaround. But as I mentioned, we would like to understand a bit more about the intent here as we're not entirley sure what else this could be affecting as yet. Aside from what seems to be a performance improvment by not calculating the context everytime, which our below fix lacks. Possibly improvement would be to compare the incoming context first, we just haven't gone that far yet.
It's worth noting we are running this in an AWS lambda environment with yoga. We have tried running in singleton mode without luck and were hitting this issue #2227. Going back to operational mode works with our fix to the cached context.
workaround
function getModuleContext(
moduleId: string,
ctx: GraphQLModules.GlobalContext
): GraphQLModules.ModuleContext {
// Reuse a context or create if not available
//update: don't use the cache
//if (!contextCache[moduleId]) {
// We're interested in operation-scoped providers only
// const providers = modulesMap.get(moduleId)?.operationProviders!;
//update: get all the providers not just the ones for this module
const providers = flatten(Array.from(modulesMap.values()).map(a => a.operationProviders));
// Create module-level Operation-scoped Injector
const operationModuleInjector = ReflectiveInjector.createFromResolved({
name: `Module "${moduleId}" (Operation Scope)`,
providers: providers.concat(
ReflectiveInjector.resolve([
{
provide: CONTEXT,
useFactory() {
//update: always inject the latest context not the cached context
return ctx
// return contextCache[moduleId];
},
},
])
),
// This injector has a priority
parent: modulesMap.get(moduleId)!.injector,
// over this one
fallbackParent: operationAppInjector,
});
// Same as on application level, we need to collect providers with OnDestroy hooks
registerProvidersToDestroy(operationModuleInjector);
contextCache[moduleId] = merge(ctx, {
injector: operationModuleInjector,
moduleId,
});
//}
return contextCache[moduleId];
}
Any assistance or thoughts would be greatly appreciated
Hey @fridaystreet, thanks for reporting! Can you try out graphql-modules@2.2.1-alpha-20231102140257-e36ba0ac please (it fixes #2227)? #2461 has potential to fix your issue too. Thanks!
Closing due to staleness.
@enisdenjo This isn't actually fixed. We had just been using our customised code above. Apologies I assumed it was using the latest and was working, but we just went to make some changes and upgrade graphql-modules and this reared it's head again. Now we can't seem to apply the previous fix to the new version of graphql-modules.
I thought it was to do with yoga as we are using yoga and we are using the yoga contextBuilder in order to put the injector into the context as this doesn't seem to happen with the useGraphQLModules plugin. I figured maybe this was getting in the way so tried removing the use of context builder, but now it's broken the directives which are trying to use the injector from the context.
As it currently stands, there does not seem to be an injector instance in the context of any directives?
Can anyone advise if this is by design and if so what is the alternative approach?
Cheers