expediagroup / graphql-component Goto Github PK

Readme is somewhat sparse.

Potentially just through converting to a gasp typscript project.

imports: [
        {
          component: componentA,
          exclude: ['Query.*', 'Mutation.*]
        },

Exclusion check does not iteration against excludes

Instead of exporting an instance, require extending GraphQLComponent such that construction is forced (which allows the implementor to have instance data / state).

Instead of namespacing context additions, should it just be flat?

Enable federation behind a flag and buildFederatedSchema instead of makeExecutableSchema.

In a component that is using the custom scalar (Like graphql-iso-date) in the following way:

const GraphQLComponent = require('graphql-component')
const { GraphQLDate } = require('graphql-iso-date')

class SomeComponent extends GraphQLComponent {
  constructor () {
    const types = `
    scalar Date
  `
    const resolvers = {
      Date: GraphQLDate,
    }
    super({ types, resolvers })
  }
}

module.exports = SomeComponent

...the following error occurs:

graphql-component:schema creating component +0ms
  graphql-component:resolver memoized DateTime.name +0ms
/Users/cholmok/work/holmok.com/node_modules/graphql-component/lib/resolvers.js:68
      wrapped[name][resolverName] = memoize(name, resolverName, func.bind(bind));
                                                                     ^

TypeError: func.bind is not a function
    at wrapResolvers (/Users/cholmok/work/holmok.com/node_modules/graphql-component/lib/resolvers.js:68:70)
    at new GraphQLComponent (/Users/cholmok/work/holmok.com/node_modules/graphql-component/lib/index.js:32:23)
    at new BlogComponent (/Users/cholmok/work/holmok.com/server/components/blog-component.js:68:5)
    at Object.<anonymous> (/Users/cholmok/work/holmok.com/server/components/index.js:20:14)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)

It appears this breaks the memoize which is assuming all the values in the resolvers object are functions and breaks when it is not.

Given a schema that contains an enum type such as:

enum Status {
  APPROVED
  PENDING
  REJECTED
}

With no internal remapping declared, the values of the enum type in resolvers (and returned to clients) are the strings 'APPROVED', 'PENDING', 'REJECTED'.

But Apollo allows the implementer to remap enum values to a different value for use within resolvers (ultimately as a way to easier interface with another system or library who may expect different values than the default stringified version of the enum field). The external API remains unchanged in terms of the value received for that enum type. Remapping the value of enums in a resolver via the resolver map which gets passed to makeExecutableSchema({resolvers}) is accomplished like this:

const resolvers = {
  ...,
  Status: {
    APPROVED: 1,
    PENDING: 2,
    REJECTED: 3
  },
  ...
};

The problem is the wrapResolvers() function is expecting the field of a type in the resolver map to map to a function in order to bind this from the GraphQLComponent class: https://github.com/tlivings/graphql-component/blob/master/lib/resolvers.js#L76 .

In the case of an enum being internally remapped to a new value, it wont and can't be a function to work properly, thus a stack trace during GraphQLComponent bootstrapping:

UnhandledPromiseRejectionWarning: TypeError: func.bind is not a function at wrapResolvers
...

The proposed solution is simple. A basic check if the func is an instance of a function. A PR linked to this issue is coming shortly.

Mocks aren't propegating down the component tree

Hello team, found an issue with trivial resolvers and the resolver wrapping to prevent double executions in delegateToComponent situations. I don't fully understand this double execution issue that relevant code tries to prevent to propose a solution, so I thought I'd create an issue first to discuss.

Current behavior

The resolution for a graph breaks when on a root object a field is present but with a different type than expected in the schema. Example:

given a type that has a field name of type String, a root object:

{
    ...
    name: {
        value: "name"
    }
    ...
}

and field resolver:

name(obj) {
    return obj.value
}

These lines will find that there's already the field name present in the root and return that, breaking the resolution because we'd be returning an object for a String type.

Expected behavior

Instead, we'd want the resolution of the type to be passed through to the trivial resolver and the resolution to not break.

Maybe solutions

A naive first approach to a solution would be to do a type check (this field expects a string... is the field already a string? return, else pass through to resolver) but the issue remains: even if the expected type matches the type we find, there could be some transformation applied in a trivial resolver (upper case a string, + 1 a zero-based index, I don't know). So maybe there's a better way for a component to know that it is in a delegateToComponent context or to provide an option to explicitly request a resolver NOT to be executed? (maybe an option to pass a list of "noDoubleExecuteResolvers" to compare against)? thoughts?

Instead of imports being one list, differentiate between imports used for typeDefs and imports you want to generate delegates to rootTypes for.

It would be cleaner if the component explicitly declared which queries and mutations to include, rather than what to exclude. that is a cleaner mental model for the component since you are stating what you want rather than filtering out what you dont. an empty config should just include all

Providing better support for the data source abstraction (or another similar abstraction) can keep components more agnostic to how data is resolved.

Adding this file at the root under index.d.ts will make typescript users happy ;-)

Tks

export declare class GraphQLComponent {
_schema: any;
_types: any[];
_resolvers: any;
_imports: any[];
_directives: any;
_context: any;
_importedTypes: any[];
_importedResolvers: any;
_useMocks: boolean;
_importedMocks: any;
_mocks: any;
_preserveTypeResolvers: boolean;
_fragments: any;
_mergedTypes: any;
_mergedResolvers: any;
constructor({ types, resolvers, imports, mocks, directives, context, useMocks, preserveTypeResolvers }?: {
types?: any[];
resolvers?: {};
imports?: any[];
mocks?: any;
directives?: any;
context?: any;
useMocks?: boolean;
preserveTypeResolvers?: boolean;
});
static isComponent(check: any): any;
execute(input: any, { root, context, variables }?: {
root?: any;
context?: {};
variables?: {};
}): Promise;
readonly schema: any;
readonly context: {
(arg: any): Promise;
use(name: any, fn: any): void;
};
readonly types: any[];
readonly importedTypes: any[];
readonly resolvers: any;
readonly importedResolvers: any;
readonly imports: any[];
readonly mocks: any;
}

Apollo's buildFederatedSchema only adds built-in directives to the schema. It does not honor the schemaDirectives option. Therefore, any custom directives are ignored. See issue apollographql/apollo-feature-requests#145.

A workaround could be to use mergeSchemas to add in the schema definition after building the federated schema.

Helps improve searching in npm

Due to ardatan/graphql-tools#1003 the schema directive @memoize is lost on merge. Will start working when this becomes available.