Repository navigation
GraphQL default schema issues and potential for customisation #5777
Description
Activity
First of all, I'd like to thank you for trying the brand new GraphQL API, do this detailed analysis and share your feedback. You are very welcome to contribute to the project in discussions and PRs.
The current version of the GraphQL API is just what we considered as the minimum to run a GraphQL API on top of Parse Server, so we could get feedback from the community (like yours) and continue working. There is still a lot that can be done to leverage all the GraphQL potential into Parse Server project. We created a first short to do list here. Feel free to suggest new tasks and/or work in any of them.
I agree with most of your points. Let me comment each of them (maybe we could create a separate issue/task for each of them):
- Plug-and-Play: In this first PR we made sure to keep the same features that the current community loves in the REST API: auto-generation, schemaless, plug-and-play approach... There are for sure some tradeoffs that we can now start discussing and addressing based on the community feedback.
- Schema overload: I completely agree with you. There is already in our roadmap a task in which the developer will be able to select which classes' operations they want to be automatically generated. Then the developer should also be able to create additional queries/mutations using cloud code.
- Nested Mutations: We decided to create the "packages" to organize users, objects, etc, operations because we figured out that, with the auto generation, it would be created so many operations in addition to the large amount that Parse has by default and it would be much harder for a new develoepr to understand how it should work if not grouped by "packages". We were not aware about the risk of loosing the serial execution functionality, but I think there is no problem, since you can run something like this (I'm not sure that it will execute in series, though. Would you be willed to create a test case for this?):
mutation StockMarketAction { updateStock: objects { updateStock(objectId: "googl", fields: { stockPrice: 50 }) { updatedAt } } createStockOrder: objects { createStockOrder(fields: { type: "sell", numShares: "50", stockId: "googl" }) { objectId } } }- Coud Functions: It is already in our roadmap to include support for cloud functions in the GraphQL API.
Again, thanks for your help. Let's keep the discussion for each of the items and tackle them as a team!
Hi @davimacedo - I'd be happy to join the project and start ticking off some of the items on your todo list.
I agree about creating separate tasks for some of the issues above; overall I'm just pleased to know that you have a plan in place already and it seems that you already have considered most of the concerns I've highlighted!
Plug-and-play
This was more of an observation and I think it's the right approach to get the "MVP", so I think it can be left as is.Schema Overload
This as you said is covered by this task here. It's the one I'm most interested in getting right - are you happy to assign it to me? In terms of further discussions around it, I'm thinking the best approach is to let me create a PR with some preliminary work to let us have a base to discuss from?Nested Mutations:
So a few points here. Given that we have schema autogeneration and plan to let developers issue their own in the near future, nesting the default mutations under predefined keys is overall a really good idea - to be honest, one of my gripes with mutations in GraphQL is that of namespacing/modularity, but this has been a point of contention over a 3 year long discussion with yet no resolve. Your solution for the serial execution is a good compromise and on the face of it, does work:mutation SyncExecutionTest { first: objects { createGraphTest(fields: { name: "Numero Uno", delay: 3000 }) { createdAt } } second: objects { createGraphTest(fields: { name: "Numero Dos" }) { createdAt } } }
{ "data": { "first": { "createGraphTest": { "createdAt": "2019-07-07T11:24:56.951Z" } }, "second": { "createGraphTest": { "createdAt": "2019-07-07T11:25:00.123Z" } } } }
Logs:
// Saving "Numero Uno" // await 3000 // Saved // Saving "Numero Dos" // SavedCloud Functions:
I have a feeling that this one will be relatively straightforward in deciding where it belongs in the graph. It'll be interesting to decide whether each registered function gets its own node, or whether we have one node called "cloudFunction" that accepts the function name as a parameter.Hope this helps, I'm happy to get started on the Schema Overload issue as soon as possible.
@omairvaiyani we are very happy to have you on board! Please find my comments below.
Schema Overload
This as you said is covered by this task here. It's the one I'm most interested in getting right - are you happy to assign it to me? In terms of further discussions around it, I'm thinking the best approach is to let me create a PR with some preliminary work to let us have a base to discuss from?I assigned the task to you and I will discuss it further in your PR
Nested Mutations:
So a few points here. Given that we have schema autogeneration and plan to let developers issue their own in the near future, nesting the default mutations under predefined keys is overall a really good idea - to be honest, one of my gripes with mutations in GraphQL is that of namespacing/modularity, but this has been a point of contention over a 3 year long discussion with yet no resolve. Your solution for the serial execution is a good compromise and on the face of it, does work:I've seen this discussion about namespaces and that's why I've decided to fo with the nested mutations. So, in your opinion, should we keep the current approach?
Cloud Functions:
I have a feeling that this one will be relatively straightforward in deciding where it belongs in the graph. It'll be interesting to decide whether each registered function gets its own node, or whether we have one node called "cloudFunction" that accepts the function name as a parameter.I'd love to have both options:
- a generic mutation called
callthat receives the function name and an input object, and return an output object - a custom operation for each function in which the developer will be able to specify the schema
What do you think?
- a generic mutation called
I've seen this discussion about namespaces and that's why I've decided to fo with the nested mutations. So, in your opinion, should we keep the current approach?
I think let's keep the nesting as it's the lesser of two evils. Introducing 100+ mutations and queries all in one namespace will be a deterrent for end users trying to construct queries. Without nesting, we would also be forced to share the space between the base types and any additional types from schema stitching future. The workaround for atomicity issue will do fine for those that need it.
I'd love to have both options: a generic mutation called call that receives the function name and an input object, and return an output object a custom operation for each function in which the developer will be able to specify the schema What do you think?Given how flexible our upcoming Parse GraphQL Config is, I don't see any reason why we can't have both! A generic mutation should definitely be set as a default, just as we set the generic
create, update, destroyand similar mutations. We can decide whether to pre-load all registered cloud functions as their mutations under the namespacecloud, or whether to leave it as hidden unless configured to be added in the config. My one concern here would be registering the cloud function names which in Parse can be registered as any valid string. But special characters will cause syntax issues with graphql. For example, the function "moduleA-doThing" is perfectly acceptable in Cloud Code, but is not accepted in.graphqlfiles where a query would be constructed. A workaround would be to clean the function name before registering it as a mutation under graphql, though this may lead to confusion.Nice! It sounds we have a plan!
Talking about the function names, I'd prefer to validate the name and, in the case it does not pass, we can just log a warn and do not generate the mutation for it. It would have to be called using the generic mutation then. We could additionally have some way to enter the GraphQL mutation name. But these details I think we can discuss further when tackling this feature.
- added a commit that references this issue
on Jul 18, 2019 @omairvaiyani do you think that we've already addressed everything here and we can close this issue?
Yup, very quick turnaround I'd say!
Reacted by Antonio Davi Macedo Coelho de Castro and Tom Fox
I'd first like to congratulate the core contributors for an excellent job at introducing GraphQL into Parse. It really is a big step forward at keeping the Parse project modern and exciting for new developers.
I can see that the approach has been one of plug-n-play; all types, queries and mutations are automatically generated which means that for many teams, they can swap out many of their existing queries with GraphQL. This is pretty amazing stuff - but it does have inherent tradeoffs. I'll list these concerns out in hopes of some productive discussion to guide future development before the community's usage of the ParseGraphQLServer becomes too reliant on the status quo.
Schema overload
Autogenerating the graph using the entire database schema, and adding all default mutations for each class is like bending GraphQL into an exact map of what Parse already offers. The real power of GraphQL is the ability to generate representations of your database that you would like to make available to your clients, incrementally and evolutionarily. Its an opportunity for enterprises and scaling teams to re-think their schema, and decide what really should be accessed and mutated.
I think an ability to configure the Parse GraphQL server should be introduced that does one or two of the following:
Now, we don't want to throw the baby out with the bathwater here. There is some impressive coding done here by the contributors around the use of resolvers and mapping of GraphQL query constraints with that of Parse. Careful consideration is needed to maintain these helpful modules whilst allowing developers to extend the ParseGraphQLServer to their specific needs.
Nested mutations lose atomicity
Mutations differ from queries in that they run in series, rather than in parallel. This is to allow atomic mutations. Unfortunately, nesting mutations as is the case with the Parse implementation, loses this synchronous execution and results in tradeoff of atomic mutations.
As it currently stands, Parse's GraphQL Schema nests all mutations under
objects(andusers) like so:No obvious location for Cloud Functions
CloudFunctions don't really have a home in mutations as it stands. This means that clients will have to maintain two separate network requests layers should they require full-usage of a parse server API. Whilst we are striving to reduce our reliance of RPC's, there are many instances where an RPC approach is required, and so we'd very much like CloudFunctions to be first-class citizens in ParseGraphQLServer.
--
I was tempted to begin forking and adding PRs to establish some of the above suggestions, but I think it's absolutely critical that some discussion is done before further code is cooked.