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

Get arguments with directive #4661

Merged
merged 6 commits into from Sep 2, 2022
Merged

Get arguments with directive #4661

merged 6 commits into from Sep 2, 2022

Conversation

nicolaslt
Copy link
Contributor

@nicolaslt nicolaslt commented Aug 19, 2022

Description

Adds a new helper function proposed in #4626.

I have also removed replaced the custom AST value parsing code with the official helper from graphql-js. It seems to be available all the way to the minimum supported peerDependency version. See this one for version v14.5.6

Related #4626

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots/Sandbox (if appropriate/relevant):

N/A

How Has This Been Tested?

Only relied on the unit tests (old and new)

Test Environment:

  • OS: Ubuntu 20.04
  • @graphql-tools/...: base of my branch
  • NodeJS: 16

Checklist:

  • I have followed the CONTRIBUTING doc and the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests and linter rules pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@changeset-bot
Copy link

changeset-bot bot commented Aug 19, 2022

🦋 Changeset detected

Latest commit: 96d5e26

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 28 packages
Name Type
@graphql-tools/utils Minor
@graphql-tools/batch-delegate Patch
@graphql-tools/batch-execute Patch
@graphql-tools/delegate Patch
@graphql-tools/graphql-tag-pluck Patch
@graphql-tools/import Patch
@graphql-tools/links Patch
@graphql-tools/load Patch
@graphql-tools/merge Patch
@graphql-tools/mock Patch
@graphql-tools/node-require Patch
@graphql-tools/relay-operation-optimizer Patch
@graphql-tools/resolvers-composition Patch
@graphql-tools/schema Patch
@graphql-tools/stitch Patch
@graphql-tools/stitching-directives Patch
@graphql-tools/wrap Patch
@graphql-tools/apollo-engine-loader Patch
@graphql-tools/code-file-loader Patch
@graphql-tools/git-loader Patch
@graphql-tools/github-loader Patch
@graphql-tools/graphql-file-loader Patch
@graphql-tools/json-file-loader Patch
@graphql-tools/module-loader Patch
@graphql-tools/prisma-loader Patch
@graphql-tools/url-loader Patch
graphql-tools Patch
federation-benchmark Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Aug 19, 2022

@nicolaslt is attempting to deploy a commit to the The Guild Team on Vercel.

A member of the Team first needs to authorize it.

@nicolaslt
Copy link
Contributor Author

nicolaslt commented Sep 2, 2022

Hmm the test suite seems to be unstable right now. I think I'll wait for a review before trying to get all-green (it seems everything that fails is unrelated to my changes)

@ardatan
Copy link
Owner

ardatan commented Sep 2, 2022

It is ok. We can merge it. Thanks @nicolaslt :)

@ardatan ardatan merged commit 403ed45 into ardatan:master Sep 2, 2022
@nicolaslt nicolaslt deleted the getArgumentsWithDirective branch September 6, 2022 10:35
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.

None yet

2 participants