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

feat(scalar): add generics to scalars #1522

Closed
wants to merge 2 commits into from

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 :)

@@ -458,8 +458,8 @@ export function getNullableType(type) {
* These named types do not include modifiers like List or NonNull.
*/
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

@@ -532,21 +532,21 @@ function resolveThunk<+T>(thunk: Thunk<T>): T {
* });
*
*/
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.

None yet

6 participants