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 type resolution by recursing prototypes #1334

Merged

Conversation

jeffutter
Copy link
Contributor

This fixes an issue when you have types declared in a Prototype (such as input_object input types for a directive). Previously type resolution would look on the Schema, but not the Prototype for the type definitions.

You could workaround this by duplicating your type definition into both the Schema and the Prototype. This makes me somewhat curious if there is a better way to fix this, in that the code that was trying to resolve the type here shouldn't be resolving it on the Schema, but rather on the Prototype 🤷. This seems to work though.

Fixes #1279

Also should fix the issue mentioned here: DivvyPayHQ/absinthe_federation#81

@jeffutter
Copy link
Contributor Author

jeffutter commented Aug 2, 2024

I just realized that the fixed test is broken with SCHEMA_PROVIDER=persistent_term (didn't realize that setting was run in CI). So I'll implement persistent_term shortly and expand the test as @kzlsakal suggested.

Done: ✅

@jeffutter jeffutter force-pushed the recurse-prototype-type-resolution branch from 77e70e2 to 60290b0 Compare August 2, 2024 17:53
@benwilson512
Copy link
Contributor

Huh I'm surprised the CI isn't running on this. Will review

@jeffutter
Copy link
Contributor Author

Huh I'm surprised the CI isn't running on this. Will review

Did the main branch change from master to main recently? It looks like CI is setup for PRs against master, I opened this against main since it seems to be the default branch.

https://github.com/absinthe-graphql/absinthe/blob/ff726c88a87d7031fc0bfb30f1a7b174168d985e/.github/workflows/elixir.yml#L3C1-L8C1

This fixes an issue when you have types declared in a Prototype (such as
input_object input types for a directive). Previously type resolution
would look on the Schema, but not the Prototype for the type
definitions.

You _could_ workaround this by duplicating your type definition into
both the Schema and the Prototype. This makes me somewhat curious if
there is a better way to fix this, in that the code that was trying to
resolve the type here shouldn't be resolving it on the Schema, but
rather on the Prototype 🤷. This seems to work though.

Fixes absinthe-graphql#1279
@jeffutter jeffutter force-pushed the recurse-prototype-type-resolution branch from 1f8122e to fad950d Compare August 20, 2024 15:56
@jeffutter
Copy link
Contributor Author

I rebased this off the latest main, CI should run once the workflow is approved.

@benwilson512 benwilson512 merged commit 3d0823b into absinthe-graphql:main Aug 20, 2024
6 checks passed
@benwilson512
Copy link
Contributor

Thanks!

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.

Introspection query fails when using a prototype schema that defines types for use by directive arguments
4 participants