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

overhaul resolver validation #53

Closed
helfer opened this issue May 17, 2016 · 9 comments
Closed

overhaul resolver validation #53

helfer opened this issue May 17, 2016 · 9 comments

Comments

@helfer
Copy link
Contributor

helfer commented May 17, 2016

  • don't enforce resolver presence at startup time, just print warnings or errors at runtime if a field without a custom resolve function is accessed and returns null.
@helfer helfer assigned helfer and unassigned helfer May 17, 2016
@rdrey
Copy link

rdrey commented May 26, 2016

It would be great if resolvers and mocks can be mixed.

I've written a pretty large schema, and tested it with mocks. It would be great if I could now start to add resolvers and any broken fields keep returning mocked values. (With warnings / errors in the console or graphQL output.)

@helfer
Copy link
Contributor Author

helfer commented May 26, 2016

Can you elaborate a bit? What do you mean by broken fields returning mocked values? Are those fields with a resolver that returns undefined, or fields without a resolver at all? I think the latter would be useful, but the former sounds like a recipe for trouble.

@rdrey
Copy link

rdrey commented May 26, 2016

The latter. I actually just saw this commit e83226a and I'm wondering if I just need to update my graphql-tools.

@helfer
Copy link
Contributor Author

helfer commented May 26, 2016

That PR was actually just fixing another issue, which is to apply the resolvers before the mocks, to make sure the mocking of union types works (union types need the __resolveType function, which is not a resolver, but still has to be provided in the resolvers argument to apolloServer)

@helfer
Copy link
Contributor Author

helfer commented May 26, 2016

I think the easiest way to get this to work is for apolloServer to pass preserveResolvers = true to addMockFunctionsToSchema inside apolloServer.js

I'd be open to a PR in that direction, if you'd like to see that feature implemented.

@rdrey
Copy link

rdrey commented May 26, 2016

This works great for me, thanks. Would you like this to be a config option? Naming suggestions?

@rdrey
Copy link

rdrey commented May 26, 2016

I think mixing mocks and resolvers kind of implies that this is what I want to do, so I'm inclined to just leave this turned on by default.

@helfer
Copy link
Contributor Author

helfer commented May 26, 2016

I think rather than making it an option (and adding more arguments to the server), I would just turn it on by default like you suggested.

I just moved all the apolloServer code to apollo-server, so can you make your PR against the apollo-server repo? It would be a one-line change here, I think. Make sure to also add a test: https://github.com/apollostack/apollo-server/blob/2a4a41982d66d4a8502f2cff6a07018a9df8f1c4/src/apolloServer.js#L87

@helfer
Copy link
Contributor Author

helfer commented Jul 8, 2016

Closing this because we moved the server and the issue no longer exists here.

@helfer helfer closed this as completed Jul 8, 2016
ardatan added a commit that referenced this issue Oct 31, 2024
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

No branches or pull requests

2 participants