No way to operate with variables in hooks

[mcollina]

Can you make an example?

[npdev453]

@mcollina

for example I want to log incoming request with operation name and variables (for debugging)

for now only one way to do this is use that hack (because req is not typed in context)

        app.graphql.addHook('preExecution', (schema, document, context) => {
                 console.log('variables', context.req.body.variables)
                 console.log('ops', readOpsNames(document)) // filtering for OperationDefinition selectionSet names
        });

[npdev453]

Also Variables is very closes, and strange for me that it no passed inside hook: https://github.com/mercurius-js/mercurius/blob/f6b6b858c24bc4bc919b25e9ac5cbf35df568d5d/index.js#L635

And also there is no way to return it modified like a Document does

(in apollo requestContext (requestDidStart) its possible)

[matijagaspar]

So if I understand you correctly @npdev453, you are saying variables should be returned separately ie: app.graphql.addHook('preExecution', (schema, document, context, variables) => { and possibly be able to return them modified:

app.graphql.addHook('preExecution', (schema, document, context, variables) => {

  return {
    modifiedVariables:  doSomethingWithVariables(variables)
  }
}

is that correct @npdev453 ?

Since variables are already included in the context via context.req would having a correctly typed context be enough?

[npdev453]

So if I understand you correctly @npdev453, you are saying variables should be returned separately ie: app.graphql.addHook('preExecution', (schema, document, context, variables) => { and possibly be able to return them modified:

app.graphql.addHook('preExecution', (schema, document, context, variables) => {

  return {
    modifiedVariables:  doSomethingWithVariables(variables)
  }
}

is that correct @npdev453 ?

Since variables are already included in the context via context.req would having a correctly typed context be enough?

I think that it should be chosen by library designer (by author @mcollina or by any active maintainer)

But in my opinion: 1) using variables from context req is not stable solution and users code can be broken after some updates (and it is not obviously to find it here) 2) ability of accessing and modifying variables should be available because hook name is preExecution and variables is part of execution (also it can be safely logged in this place with Pino and redact function). Why not? Seems like synthetic limiting. 3) context and context.req - should be typed as well, as you can absolutely know that it can be only passed from fastify - but maybe not, if context means different idea

[mcollina]

I'm kind of lost in all of this. By the look of https://github.com/mercurius-js/mercurius/issues/876#issuecomment-1259506419, this seemed an issue about lack of types and or docs.

        app.graphql.addHook('preExecution', (schema, document, context) => {
                 console.log('variables', context.req.body.variables)
                 console.log('ops', readOpsNames(document)) // filtering for OperationDefinition selectionSet names
        });

This is the proper way to access the request body. There does not seem to be a need for anything more than this.

[simoneb]

@npdev453 is this issue sorted for you? Can you close it?

[npdev453]

@npdev453 is this issue sorted for you? Can you close it?

Yeah, thank! Strange that gitlab dont close it automatically