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

Infer input fields from setFieldsOnGraphQLNodeType #2075

Merged
merged 2 commits into from
Sep 15, 2017
Merged

Infer input fields from setFieldsOnGraphQLNodeType #2075

merged 2 commits into from
Sep 15, 2017

Conversation

Zalastax
Copy link
Contributor

Makes fields added by setFieldsOnGraphQLNodeType filterable and sortable.
I didn't see any uses of setFieldsOnGraphQLNodeType in the tests so adding new tests felt like a non-starter.

Closes #1931

@KyleAMathews
Copy link
Contributor

Deploy preview failed.

Built with commit 73ebcc5

https://app.netlify.com/sites/using-glamor/deploys/59b562b8cf321c7aaed3fe17

@gatsbybot
Copy link
Collaborator

gatsbybot commented Sep 10, 2017

Deploy preview ready!

Built with commit 5127fa1

https://deploy-preview-2075--gatsbygram.netlify.com

@KyleAMathews
Copy link
Contributor

Hey! From a quick read through everything looks very sensible & workable. Thanks!

Would love some tests for the new functionality. The reason there's not any existing tests is core to this point has had nothing to do with setFieldsOnGraphQLNodeType so if things failed, it was on the userland code to handle it. The GraphQL schema inference parts of the code are complex & core to Gatsby so we need to keep them very well tested.

@Zalastax
Copy link
Contributor Author

Good, no problem. It would really help if you can show me how to add usage of setFieldsOnGraphQLNodeType to the tests, if you have the time.

@KyleAMathews
Copy link
Contributor

@Zalastax awesome! Basically what you want to do is simulate creating various schemas with different GraphQL types usingsetFieldsOnGraphQLNodeType and assert that the correct connection input fields are being created.

Some of the tests in here should be a good guide: https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby/src/schema/__tests__

@KyleAMathews
Copy link
Contributor

Happy to answer more questions about the process as you dive in!

@Zalastax
Copy link
Contributor Author

How do I mock api-runner-node? If my test can provide the result of that call, then the rest is a piece of cake. I could of course change the code to instead provide the result of await apiRunner(`setFieldsOnGraphQLNodeType` to createType, but it seems nicer to create some fake plugins.

@KyleAMathews
Copy link
Contributor

No need to mock it, that's overkill. Just manually add additional GraphQL fields that aren't in the node data.

@Zalastax
Copy link
Contributor Author

@KyleAMathews
Copy link
Contributor

There's nothing special about that code. It just calls plugins and gets an array of graphql fields. You're testing the inference code not how gatsby gets fields from plugins.

* Add tests
* Strengthen error recovery for objects
@Zalastax
Copy link
Contributor Author

@KyleAMathews I'm quite happy with this now 😃. Tell me if you need something more before merging.

@KyleAMathews
Copy link
Contributor

Hey this looks really really good! Thanks for adding this! Some tricky code but definitely fills in a big missing piece to Gatsby's GraphQL story 🙏

@jlengstorf
Copy link
Contributor

Hiya @Zalastax! 👋

This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here.

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (We’ve got t-shirts and hats, plus some socks that are really razzing our berries right now.)
  2. If you’re not already part of it, we just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again! 💪💜

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.

4 participants