Enables support for resolver extensions for compatibility with grafast, complexity, etc. #2618
Conversation
vlad-x
force-pushed
the
feature/resolver-extensions
branch
from
d9f7fd2 to
aa5b092
Compare
| if (isDefined((obj[fieldName] as any).extensions)) { | ||
| // some extensions allow to omit the resolve function, e.g. grafast | ||
| const defaultResolver = (val: any) => val; | ||
| const resolver = wrapResolver({ | ||
| config, | ||
| resolver: (obj[fieldName] as any).resolve || defaultResolver, | ||
| middlewareMap, | ||
| path, | ||
| }); | ||
| resolvers[typeName][fieldName].resolve = resolver; | ||
| } | ||
|
|
There was a problem hiding this comment.
Is this needed? It doesn't seem to do anything with the extensions, and adding a resolver where one isn't needed unnecessarily increases memory usage of the schema (and will also break Grafast plans).
| if (isDefined((obj[fieldName] as any).extensions)) { | |
| // some extensions allow to omit the resolve function, e.g. grafast | |
| const defaultResolver = (val: any) => val; | |
| const resolver = wrapResolver({ | |
| config, | |
| resolver: (obj[fieldName] as any).resolve || defaultResolver, | |
| middlewareMap, | |
| path, | |
| }); | |
| resolvers[typeName][fieldName].resolve = resolver; | |
| } |
There was a problem hiding this comment.
@benjie thanks for the review! 🙌
Yes, this is needed due to how graphql-modules works internally - it needs to have some sort of resolver to keep track of this part of schema.
An implementation without a resolver would require a much bigger refactoring of graphql-modules, my goal here was to make it work with minimal changes.
| @@ -2,6 +2,7 @@ import 'reflect-metadata'; | |||
| import { makeExecutableSchema } from '@graphql-tools/schema'; | |||
| import { createApplication, createModule, testkit, gql } from '../src'; | |||
| import { NonDocumentNodeError } from '../src/shared/errors'; | |||
| import { resolve } from 'path'; | |||
There was a problem hiding this comment.
| import { resolve } from 'path'; |
Did you import this by accident?
| import MyQueryType from './query.type.graphql' | ||
| import { createModule } from 'graphql-modules' | ||
| import { loadFilesSync } from '@graphql-tools/load-files' | ||
| import { join } from 'path' | ||
| import { constant } from "grafast"; |
There was a problem hiding this comment.
I don't think you meant to make these changes?
Description
This change makes it possible to use graphql-modules with resolver extensions such as grafast, graphql-query-complexity and others.
Fixes #2615
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Environment:
@graphql-modules/...: 3.0.0Checklist:
Further comments
This is a pretty trivial change, but will significantly improve compatibility of graphql-modules with the rest of graphql ecosystem.