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

fix generating incorrect type for custom scalars #1677

Closed
wants to merge 2 commits into from

Conversation

Tiedye
Copy link

@Tiedye Tiedye commented Sep 27, 2023

Motivation

#1571 caused the graphql dsl compiler to return the incorrect type for custom scalars. Custom scalars define two methods coerce_input and coerce_result, in particular coerce_result generates the data that is returned to the mutation and this can be any type. T.untyped was correct

Implementation

Revert the offending change in #1571

Tests

Updated the existing tests

@paracycle
Copy link
Member

I think instead of removing the whole Module fallback, what we really need to do is handle GraphQL::Schema::Scalar types explicitly right before that to generate T.untyped. Would that not fix the CustomScalar problem?

@Tiedye
Copy link
Author

Tiedye commented Sep 28, 2023

I think instead of removing the whole Module fallback, what we really need to do is handle GraphQL::Schema::Scalar types explicitly right before that to generate T.untyped. Would that not fix the CustomScalar problem?

I'll give it a go

@Tiedye
Copy link
Author

Tiedye commented Sep 28, 2023

I think instead of removing the whole Module fallback, what we really need to do is handle GraphQL::Schema::Scalar types explicitly right before that to generate T.untyped. Would that not fix the CustomScalar problem?

Yep that worked

@Tiedye
Copy link
Author

Tiedye commented Oct 3, 2023

@paracycle How's that look?

@nicoco007
Copy link
Contributor

I think instead of removing the whole Module fallback, what we really need to do is handle GraphQL::Schema::Scalar types explicitly right before that to generate T.untyped. Would that not fix the CustomScalar problem?

Oh, I did something similar in #1659 that I think addresses this.

@Tiedye
Copy link
Author

Tiedye commented Oct 4, 2023

I think instead of removing the whole Module fallback, what we really need to do is handle GraphQL::Schema::Scalar types explicitly right before that to generate T.untyped. Would that not fix the CustomScalar problem?

Oh, I did something similar in #1659 that I think addresses this.

Nice! Yours is better, I'll close this if that gets merged

@andyw8 andyw8 removed their request for review October 12, 2023 16:09
@Tiedye Tiedye closed this Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants