Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not add default scalar if already exists #193

Closed
zhenwenc opened this issue May 15, 2019 · 10 comments
Closed

Do not add default scalar if already exists #193

zhenwenc opened this issue May 15, 2019 · 10 comments
Labels

Comments

@zhenwenc
Copy link

zhenwenc commented May 15, 2019

Motivation

I am planning to use graphql-compose to enhance type-graphql generated schema.

import { SchemaComposer } from 'graphql-compose';
import { buildSchema } from 'type-graphql';

const generatedSchema = buildSchema(...); // returns GraphQLSchema
const composer = new SchemaComposer(generatedSchema);
...

The generated schema already includes a set of GraphQLScalarType, which are duplicated with the set of default scalars.

However, I would suggest do not provide default scalar types at all, since I would expect this package provides helper functions to enhance my existing schema, but not changing any definitions implicitly. For instance, my schema may expect a different DateScalarType parse and/or serialise implementation.

@nodkz nodkz closed this as completed in 8066134 May 16, 2019
@nodkz
Copy link
Member

nodkz commented May 16, 2019

Great point! Thanks!

Please try the new version, it will be published from minute to minute.

@nodkz
Copy link
Member

nodkz commented May 16, 2019

🎉 This issue has been resolved in version 7.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nodkz nodkz added the released label May 16, 2019
@zhenwenc
Copy link
Author

Thanks a lot for the very quick fix! 🎉🍻

@zhenwenc
Copy link
Author

Hi @nodkz , I think the problem still exists.

By printing the composer.types value, I got:

      String => ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: String,
        _gqcSerialize: [Function: serializeString],
        _gqcParseValue: [Function: coerceString],
        _gqcParseLiteral: [Function: parseLiteral] },
      'String' => ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: String,
        _gqcSerialize: [Function: serializeString],
        _gqcParseValue: [Function: coerceString],
        _gqcParseLiteral: [Function: parseLiteral] },
      ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: String,
        _gqcSerialize: [Function: serializeString],
        _gqcParseValue: [Function: coerceString],
        _gqcParseLiteral: [Function: parseLiteral] } => ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: String,
        _gqcSerialize: [Function: serializeString],
        _gqcParseValue: [Function: coerceString],
        _gqcParseLiteral: [Function: parseLiteral] },

@nodkz
Copy link
Member

nodkz commented May 17, 2019

Schema keeps one type in Registry several times under the different keys - type name as string, type as GraphQL, type as TypeComposer. It helps to get the same type under different keys and not duplicate them.

So your output log is correct.

Maybe problem somewhere else. What specific case does not work for you?

@zhenwenc
Copy link
Author

@nodkz Sorry for the late reply.

You are correct, I have miss-looked the composer.types keys, sorry about that. The composer is complaining about:

Schema must contain uniquely named types but contains multiple types named \"JSON\".

But I have checked that the composer only have these types related to JSON:

      JSON => ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: JSON,
        _gqcSerialize: [Function: serialize],
        _gqcParseValue: [Function: parseValue],
        _gqcParseLiteral: [Function: parseLiteral] },
      'JSON' => ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: JSON,
        _gqcSerialize: [Function: serialize],
        _gqcParseValue: [Function: parseValue],
        _gqcParseLiteral: [Function: parseLiteral] },
      ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: JSON,
        _gqcSerialize: [Function: serialize],
        _gqcParseValue: [Function: parseValue],
        _gqcParseLiteral: [Function: parseLiteral] } => ScalarTypeComposer {
        schemaComposer: SchemaComposer,
        _gqType: JSON,
        _gqcSerialize: [Function: serialize],
        _gqcParseValue: [Function: parseValue],
        _gqcParseLiteral: [Function: parseLiteral] },

And here is the full list ([...composer.types.values()].map(v => v.getTypeName())):

[     'Query',
      'String',
      'String',
      'String',
      'SampleObject',
      'SampleObject',
      'SampleObject',
      'JSON',
      'JSON',
      'JSON',
      'Mutation',
      'SampleInput',
      'SampleInput',
      'SampleInput',
      'Boolean',
      'Boolean',
      'Boolean',
      'Query',
      'Mutation' ]

@zhenwenc
Copy link
Author

zhenwenc commented May 19, 2019

I have confirmed that there are two versions of JSON types had been passed to the new GraphQLSchema function, only one of them was internal to my code.

@zhenwenc
Copy link
Author

zhenwenc commented May 19, 2019

Looks like it was coming from the default directive.

I have renamed my JSON scalar to JSONX (to avoid the conflict), the schema built successfully, here is the produced GraphQL schema (with printSchema in graphql-js):

"""Provides default value for input field."""
    directive @default(value: JSON!) on INPUT_FIELD_DEFINITION

    """
    The `JSON` scalar type represents JSON values as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf).
    """
    scalar JSON

    """
    The `JSON` scalar type represents JSON values as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf). Foo!
    """
    scalar JSONX

    type Mutation {
      sampleMutation(input: SampleInput!): JSONX!
    }

    type Query {
      sampleQuery(value: String!): SampleObject!
    }

    input SampleInput {
      foo: String
    }

    type SampleObject {
      foo: String
    }

@nodkz
Copy link
Member

nodkz commented May 19, 2019

You may add your JSON to the schemaComposer.add(YourJSONInstanceType) before starting tests or constructing types. In such case will be used your type.

@zhenwenc
Copy link
Author

Thanks very much for the help! 🎉🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants