RewriteHandler state variables

[ChrisLahaye]

Hello!

Would it be possible to expose some of the variables

  private rewriters: Rewriter[];
  private matches: RewriterMatch[] = [];
  private hasProcessedRequest: boolean = false;
  private hasProcessedResponse: boolean = false;

such that it is possible to determine if rewrites have been performed?

Thank you!

[chanind]

That sounds like a reasonable request, maybe we could add something like a hasMatches getter which returns a boolean if this handler matched or not? it also might make sense to add an onMatch callback to each individual rewriter, so you could see which specific rewriter matched and log something if needed. So, something like:

new FieldArgTypeRewriter({
      fieldName: 'userById',
      argName: 'id',
      oldType: 'String!',
      newType: 'ID!',
      onMatch: (query, variables, path) => void,
}),

Are you using this inside of express with https://github.com/ef-eng/express-graphql-query-rewriter? It might make sense to add an onMatch and onRewrite callback to there too, since the RewriteHandler isn't directly exposed when using that middleware.

I'll think some more about this and put up a PR within the next week or so

[ChrisLahaye]

@chanind I am using RewriteHandler directly in a middleware made for koa. Personally I would be helped by having access to matches since I need to know which paths were modified by which rewriter.

[chanind]

:tada: This issue has been resolved in version 1.4.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[ChrisLahaye]

@chanind thanks!

Could you perhaps do the same for the variables in a rewriter? Currently only methods are exposed.

FieldArgTypeRewriter {
  rootTypes: [ 'query', 'mutation', 'fragment' ],
  fieldName: 'getHomeEvents',
  matchConditions: undefined,
  argName: 'tense',
  oldTypeNode: {
    kind: 'NamedType',
    name: { kind: 'Name', value: 'QueryGetHomeEventsTense', loc: [Object] },
    loc: { start: 0, end: 23 }
  },
  newTypeNode: {
    kind: 'NamedType',
    name: { kind: 'Name', value: 'EventTense', loc: [Object] },
    loc: { start: 0, end: 10 }
  },
  coerceVariable: [Function: identifyFunc]
}

Would be nice to have fieldName, oldType and newType available.

[chanind]

What's the usecase for exposing all rewriter variables publicly? The current setup is such that all rewriters implement the same interface, so you can always call the same few methods uniformly on any rewriter to rewrite the response. Is exposing these variable just for logging purposes when something matches?

[ChrisLahaye]

What's the usecase for exposing all rewriter variables publicly? The current setup is such that all rewriters implement the same interface, so you can always call the same few methods uniformly on any rewriter to rewrite the response. Is exposing these variable just for logging purposes when something matches?

Correct. We would like to log clients that use an out-dated schema and have information about which rewrites were performed.

[chanind]

Would it work to add a standard onMatch callback to all the rewriters instead? I'm worried it will be hard to have a standardized way to expose internals of each rewriter without making every minor internal change to rewriters into a potentially breaking change to the public interface. That way, you could log info about each specific rewriter flexibly too, like:

new FieldArgTypeRewriter({
      fieldName: 'userById',
      argName: 'id',
      oldType: 'String!',
      newType: 'ID!',
      onMatch: (query, variables, path) => {
        logger.info(`userById String! -> ID! type rewrite`, { path, query });
     },
}),

[ChrisLahaye]

@chanind why not add an onMatch callback to the rewrite handler that emits the path and rewriter initialization object?

[chanind]

That works too, I'm not opposed to adding that method. But, I don't want to have to expose all the internal properties of every rewriter publicly, since then it's impossible to change any internal implementation of the rewriters without considering it a breaking change.

[chanind]

What do you think about allowing setting an optional label param on each rewriter that could be exposed publicly? That way if you want to differentiate which rewriter matched in an onMatch callback for the rewriter handler, you could set a different label for each rewriter and read it out in the onMatch callback, and wouldn't require exposing all the private variables of each rewriter publicly.