Provide way to delegate nested type resolution to resolver for that type about graphql HOT 12 CLOSED

Something I want to point out, that may not be completely clear in the docs, is that these "god class" resolvers that you have, the Viewer settings the resolve for both the configuration and the company, is not actually necessary. As Viewer has the child of Company it should set up a resolve for the company. However, as Company has the child of Configuration its resolver should set up the resolve field for the Configuration object.

I've made a very simplistic example that shows how Foo, which has a child of Bar, which has a child of Baz, which has a child of Quu, can each set up their child's direct resolver while letting the children each handle their child resolvers, so that Foo doesn't need to be aware of anything deeper than the first level.

With the above in mind, it might still be interesting to see a different approach to how it might be possible to set up resolve field references on the object itself for when it is a child of a query. I'll try to take some time to read through and comprehend the approach that you've already got theorized here and provide what feedback I can

from graphql.

Okay, looking at the playground PRs, I get the idea of what you're wanting to achieve. My personal opinion is that the current implementation is correct, in how it explicitly requires you to say how the current resolver handles its child resolutions. It moves the requirement from the child saying how to resolve itself to the parent saying how to get its child. And this, I think, makes sense, especially in cases where you need to pass information about the @Parent() to the resolver. Now, if you can do this in the proposed API as well, that's great.

Before going any further, I want to say that I really do appreciate the interest and thought put into the approach you've provided! I'm looking to make sure we're handling all the use cases we can from all the angles I can think of to make sure we don't end up missing any edge cases. Apologies if it comes off nit-picky or antagonistic, as that's not my intent.

I want to ask: with the proposed API, how do you handle when you want slightly different behavior between the way two parents resolve the same child type? Say that you have Foo and Bar both wanting to be able to resolve Baz, but you want the resulting baz to either be foobaz or barbaz depending on its parent. So a query like

query {
  foo {
    foo
    bar {
      bar
      baz {
        baz
      }
    }
    baz {
      baz
    }
  }
}

Might come out to something like

{
  "foo": {
    "foo": "this is the normal foo",
    "bar": {
      "bar": "this is the normal bar",
      "baz": {
        "baz": "barbaz"
      }
    },
    "baz": {
      "baz": "foobaz"
    }
  }
}

If this @ResolveObjectType() is used, can it handle different resolutions between parent types, or is that something that has to be implemented within the context of that specific method? I feel like that would get messy rather quickly. And if looking back at a REST context, injecting the child resolver into the parent would be the "correct" approach, if you don't plan on using a join to manage that

from graphql.

I've been playing with this locally, and I did the following to get it to "work". I'm sure there are tons of holes here, however:
in packages/graphql/lib/schema-builder/storages/type-metadata.storage.ts
update addResolverMetadata

  addResolverMetadata(metadata: ResolverClassMetadata) {
    this.metadataByTargetCollection.get(metadata.target).resolver = metadata;
    this.metadataByTargetCollection.get(metadata.typeFn()).resolver = metadata;
  }

in the same file, add a new method:

  getResolverMetadataFor(metadata: PropertyMetadata): ResolverClassMetadata {
    // there is a tsc error here, so I just tossed an any on it for now
    return this.metadataByTargetCollection.get(metadata.typeFn() as any).resolver;
  }

in packages/graphql/lib/schema-builder/factories/object-type-definition.factory.ts update createFieldResolver to look for a resolver of a type.

  private createFieldResolver<
    TSource extends object = any,
    TContext = {},
    TArgs = { [argName: string]: any },
    TOutput = any,
  >(field: PropertyMetadata, options: BuildSchemaOptions) {
    // get the the resolver for this field's type
    const typeResolver = TypeMetadataStorage.getResolverMetadataFor(field);

    const rootFieldResolver = (root: object) => {
      const value = root[field.name];
      // if typeResolver exists
      if(typeResolver) {
       /**
        * get the instance but I don't think this would be correctly scoped. I also assume
        * using moduleRef in each field resolver might be expensive.  Given these concerns, this is just to prove it 
        * some amount of possibility exists here.
        */
        const instance = this.moduleRef.get(typeResolver.target, { strict: false, });
        if(instance) {
          /**
           * make an assumption that some "common" method exists (see `resolveType` proposal from above)
           * otherwise fallback do existing possible values.
           */
          return instance['resolve']?.(root) ?? value ?? field.options.defaultValue;
        }
      }

      
      return typeof value === 'undefined' ? field.options.defaultValue : value;
    };
    const middlewareFunctions = (options.fieldMiddleware || []).concat(
      field.middleware || [],
    );
    if (middlewareFunctions?.length === 0) {
      return rootFieldResolver;
    }
    const rootResolveFnFactory = (root: TSource) => () =>
      rootFieldResolver(root);

    return decorateFieldResolverWithMiddleware<
      TSource,
      TContext,
      TArgs,
      TOutput
    >(rootResolveFnFactory, middlewareFunctions);
  }
}

from graphql.

What I've essentially come to as a "what needs to happen" conclusion is that we need a way to replace the "default" resolvers that are assigned during the schema building step with a method that ultimately wraps the child objectType's resolver.

When we look at the schema that's generated in graphql-federation.factory.ts with a debugger, we can peek at the _typeMap which defines every type. Each type there has a _fields object which defines all of your fields and their resolvers. Anywhere we have a field with a type of GraphQLObjectType, we need to be able to replace/delegate it's resolve function with a method that leverages the resolver for that type. In cases where we defined a field resolver, with @ResolveField, we have a function named "resolverCallback" otherwise we have a resolver called rootFieldResolver. Where either of those is defined, we need a way to get at scoped resolvers for the corresponding type and replace the resolver with the annotated @Resolver for that type.

from graphql.

I put up a relatively incomplete draft PR as far as the community standards go, but I hope it helps with some inspiration/guidance for myself https://github.com/nestjs/graphql/pull/3026/files

from graphql.

@shohoney - I'm not at all experienced with federation. But, I've done some research in the past.

Is this the kind of problem you are wanting to solve and does Nest not offer it?

https://medium.com/volvo-car-mobility-tech/working-with-entities-in-graphql-federation-a88e9f9865b2#:~:text=Querying%20an%20Entity%20in%20a%20subgraph&text=Normally%2C%20a%20query%20in%20GraphQL,order%20to%20fetch%20any%20entity.&text=Thus%2C%20every%20federated%20subgraph%20has%20to%20be%20able%20to%20resolve%20Query.

Scott

from graphql.

While a federated grap(h) surfaced the issue for us, this isn't actually a federation problem. This is ultimately a separation of concerns issue. I create resolvers for every type of my graph. I want those resolvers to handle the resolution of their respective types. Not for some parent resolver to suddenly usurp the responsibility because the framework simply doesn't delegate resolution down to the relevant resolver. I'm here with the aim of correcting that. From what I can tell, NestJS does not do this, at least not by default or by understood convention. There are some simple hacks above that accomplish this, as I mentioned above, but all are incomplete. My rough PR shows a different way of accomplishing this, but is also incomplete. That said, I feel that my solution is closer to the spirit of resolver chains than how nest handles it today.

from graphql.

I feel that my solution is closer to the spirit of resolver chains

Where did you get your current understanding of the "spirit of resolver chains"? GraphQL (with Nest or without) has been like this for many years with nobody complaining. Could it possibly be your understanding on this is a bit "off"?

Above and beyond that, if you need that very niche structure of grabbing deeper and deeper non-hierarchical data to make it hierarchical, couldn't you:

1. Inject the service that offers the data (since it's not a federation issue???) into the resolver?

2. you have your service return (merge) the data as needed?

3. if it is a federation issue, check to see if Nest does offer what I pointed out in the article? I bet it does, because it comes from Apollo Federation and Nest rarely, if at all, mutes features of the packages it wraps.

4. have your UI component query for the data it needs, outside of the business graph (like getting a config from a company???)? I.e. You don't have to pump all data for a page through one query and make it all "fit". In fact, that would be pretty silly. You can have multiple queries to get a single page loaded. In most cases, you will.

And above and beyond all that, can you show any other GraphQL implementations (in other languages) that do what you are requesting? It might help your argument for the change.

Scott

from graphql.

@jmcdo29 , thanks for the example. We've definitely done some of this, and it's where we start seeing providers getting a little unwieldy, IMO. Especially where you have a large root type that resolves several types from a bunch of unrelated modules/providers. I've opened 2 PRs on my playground repository which I hope shed light on what the ultimate goal is here.

Please note, the first PR has some unrelated stuff from a previous thing I was tinkering on. you'll primarily want to focus on the foo bar and baz folders. however, I also have viewer company and configuration available for querying:

{
  foo {
    foo
    bar {
      bar
      baz {
        id
        baz
      }
    }
  }
}

and for teh viewer root

{
  viewer {
    fullName
    company {
      name
      someProductConfig{ 
        name
      }
    }
  }
}

Without my proposal: shohoney/playground#1
with the proposal from #3026 : shohoney/playground#2

If you want to pull it down, I think you should be able to just clone on each branch and do a yarn install. If you have issues for some reason, let me know I'm happy to help.

from graphql.

Thanks for the feedback! I will address what I think are the two main questions. Please let me know if I'm misunderstanding anything:

Okay, looking at the playground PRs, I get the idea of what you're wanting to achieve. My personal opinion is that the current implementation is correct, in how it explicitly requires you to say how the current resolver handles its child resolutions. It moves the requirement from the child saying how to resolve itself to the parent saying how to get its child. And this, I think, makes sense, especially in cases where you need to pass information about the @parent() to the resolver. Now, if you can do this in the proposed API as well, that's great.

If you look at my draft, you'll notice on object-type-definitions.factory.ts:235, we pass the whole list of resolver arguments. In the implementation of rootFieldResolver we have access to those parameters, and you'll notice I'm using the GqlParamsFactory to get the parameters. I just couldn't sort out how to actually get the parameter decorators to work. I assume someone with more knowledge of the schema building steps would be able to help correct this issue. in my comment on the draft PR, I note that I think I might need to use externalContextCreator but I wasn't sure and expected that I'd get some guidance from y'all on that if we decided to take this from POC to more fully developed.

If this @ResolveObjectType() is used, can it handle different resolutions between parent types, or is that something that has to be implemented within the context of that specific method? I feel like that would get messy rather quickly. And if looking back at a REST context, injecting the child resolver into the parent would be the "correct" approach, if you don't plan on using a join to manage that

If I'm understanding this question correctly, I think you're just wondering whether or not we can effectively override the @ResolveObjectType in cases where we want different resolution behavior? Given your example above there is a "default" behavior, but in the case of baz being a child of foo, we want different behavior. What I've built here will prefer @ResolveField over @ResolveObjectType. Think of @ResolveObjectType as a "default" way to resolve the object. If you want to have that, as well as different behaviors in other areas, it's possible. shohoney/playground#3

Ultimately, I think this exists as an extra tool in the box, rather than being something that's intended to entirely override the behaviors that exist today.

from graphql.

I think maybe one of the questions you're getting at that I didn't address in regards to the second question is more around how a type may resolve differently relative to the parent args that exists. This is a valid issue. When I was thinking about it, I was largely thinking of this as a similar idea to the referenceResolver that's used with federated graphs. That referenceResolver gets information about the inbound type so it knows how to resolve itself based on that. In the context of this, it's a bit more complicated. I'll think about it some and see if I can reason around a sensible solution for that.

from graphql.

Thanks for your suggestion!

There are no plans to implement it in the foreseeable future.

If you think your request could live outside Nest's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

from graphql.