Skip to content

Conversation

@sibelius
Copy link

based on gtkatakura/fullstack-challenge@36cc105

and #1487
and #576
and #914

only 2 flow erros to fix

image

for the first one, we can do this

this.parseValue = config.parseValue || (value => (value: any): TInternal);

for the second one, we can add a type param to valueFromASTUntyped or try to type cast the function (not sure if this is possible)

@sibelius
Copy link
Author

fixed this 2 issues

now, we got 20 more to fix

like this one:

if (isObjectType(parentType) || isInterfaceType(parentType)) {
          fieldDef = parentType.getFields()[fieldName];
        }

I think %checks refinements does not work well with generics

it does not recognize that parentType can't be null in this situation

Copy link

@Neitsch Neitsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love strong typing! Thank you for looking into this :)

export type GraphQLNamedType =
| GraphQLScalarType
| GraphQLObjectType
| GraphQLScalarType<any, any>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be <*, *>?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • is deprecated on flow type

*
*/
export class GraphQLScalarType {
export class GraphQLScalarType<TInternal = any, TExternal = TInternal> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what the purpose of / difference between TInternal and TExternal is?

@IvanGoncharov IvanGoncharov added this to the v15.0.0 milestone Feb 5, 2019
@IvanGoncharov IvanGoncharov removed this from the v15.x.x milestone Aug 13, 2020
Base automatically changed from master to main January 27, 2021 11:10
@IvanGoncharov
Copy link
Member

Fixed by #3228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants