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

Draft: Update Resolvers type to support Scalar #808

Closed
wants to merge 2 commits into from

Conversation

Alx101
Copy link

@Alx101 Alx101 commented Sep 7, 2022

Description

Fixes missing type definition (GraphQLScalarType) for Resolvers in makeExtendSchemaPlugin.ts
Discord

Performance impact

unknown

Security impact

unkown

Checklist

  • My code matches the project's code style and yarn lint:fix passes.
  • I've added tests for the new feature, and yarn test passes.
  • I have detailed the new feature in the relevant documentation.
  • I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
  • If this is a breaking change I've explained why.

@benjie
Copy link
Member

benjie commented Sep 7, 2022

Thanks for your contribution! To merge this as an officially supported interface we need a test that ensures it works. Doing so isn't too complex:

  1. Create yourself a plugin using this functionality; here's an example ToyCategoriesPlugin we use in the tests: https://github.com/graphile/graphile-engine/blob/v4/packages/postgraphile-core/__tests__/integration/ToyCategoriesPlugin.js
  2. Add a config boolean that enables your plugin; here's the one we use with ToyCategoriesPlugin:
    config.ToyCategoriesPlugin ? ToyCategoriesPlugin : null,
  3. Add a test GraphQL file that uses this functionality, it must end in .test.graphql and it must contain the special #> MyPluginNameHere: true config line; here's an example for the ToyCategoriesPlugin:
  4. Be sure that your GraphQL test file actually queries your scalar!

Once that's done, run the tests via UPDATE_SNAPSHOTS=1 yarn test inside the packages/postgraphile-core folder and your GraphQL query will be executed and the relevant snapshots will be created - check that they're sensible, and if so, go ahead and commit them.

@benjie
Copy link
Member

benjie commented Sep 27, 2023

Going to close this for now; feel free to re-open it with a test ❤️

@benjie
Copy link
Member

benjie commented Sep 27, 2023

Supported in V5: graphile/crystal#1760

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.

2 participants