[flow] Add generics for scalar class

[Slessi]

Disclaimer: Was asked to submit a PR here in order to match my PR at DefinitelyTyped/DefinitelyTyped#28419

Basically I want to add the option to specify generics on GraphQLScalarType, and not just GraphQLScalarTypeConfig

Not sure how to resolve the following issue in flow though

Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/type/definition.js:49:5

Cannot use GraphQLScalarType [1] without 0-2 type arguments.

      46│  * These are all of the possible kinds of types.
      47│  */
      48│ export type GraphQLType =
      49│   | GraphQLScalarType
      50│   | GraphQLObjectType
      51│   | GraphQLInterfaceType
      52│   | GraphQLUnionType
        :
 [1] 535│ export class GraphQLScalarType<TInternal = any, TExternal = any> {

I would not want to create a change that forces users to add <> to their code - Is there a way in flow to make the generics totally optional as in TypeScript?

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[sibelius]

@Slessi can you check this as well gtkatakura/fullstack-challenge@36cc105

export const GraphQLInt: GraphQLScalarType<number>;
export const GraphQLFloat: GraphQLScalarType<number>;
export const GraphQLString: GraphQLScalarType<string>;
export const GraphQLBoolean: GraphQLScalarType<boolean>;
export const GraphQLID: GraphQLScalarType<string>;

following this approach we could use mapped types in Typescript to transform GraphQLInt to number

[Slessi]

@sibelius I don't understand what you are trying to tell me.

I made a PR to @types/graphql with the same changes at DefinitelyTyped/DefinitelyTyped#28419 but it was rejected because it needs to match the types here

[sibelius]

you PR does not add correct types for GraphQLInt for instance

[Slessi]

Yes but what difference does that make in practice? If you're making a custom scalar resolver, you can just make a new type, i.e.

const ID = new GraphQLScalarType<number, string>({
    name: "ID",
    description: "ID Object",
    serialize: (value) => String(value),
    parseValue,
    parseLiteral: (valueNode, variables) => {
        if (valueNode.kind !== "StringValue") {
            throw new Error("Expected StringValue");
        }

        return parseValue(valueNode.value);
    },
});

[sibelius]

you can use Mapped Types on Typescript to transform GraphQLInt to number, no need to retype this on TS as well

[sibelius]

tests still failing

I think in flow default of generics is different

<TInternal: any>

or you should try

<TInternal?: any>

try running flow check locally to make it typecheck properly

[Slessi]

@sibelius I don't know how to fix it, as mentioned in OP. If you want to try yourself, feel free

[gtkatakura-bysoft]

I think this is not going to be possible ...

I would not want to create a change that forces users to add <> to their code - Is there a way in flow to make the generics totally optional as in TypeScript?

At least according to the Flow documentation:

[Slessi]

@gtkatakura-bysoft thanks for that, going to close this then as its far too trivial an improvement to merit a breaking change like that.

Unfortunate however that the maintainers of @types/graphql won't accept a PR to improve TS types due to a shortcoming of flow