graphqlcrud / spec

Specification for GraphQLCRUD
https://graphqlcrud.org
Apache License 2.0
22 stars 4 forks source link

Plularization of the queries #28

Open wtrocki opened 4 years ago

wtrocki commented 4 years ago

English language as many others have plural versions. What it means that findTask is different than findTasks operation can be come

find Object [s,ss,sh] etc. Having plurals brings confusion to the spec - especially when developers dealing with models that might not be easily pluralized.

Current implementations in javascript using pluralize library that has some specific rules but other languages will have different ones.

We are hitting this issue when building client side queries for graphback.

Solution:

  1. Do not use plural form getTask and task - as the plural query.
  2. Always add s even if the form will not be valid
  3. Others?
wtrocki commented 4 years ago

@machi1990 @craicoverflow This is something I have totally overlooked. What would be your take?

machi1990 commented 4 years ago

@wtrocki thanks for opening this issue which is well summarized.

Solution:

  1. Do not use plural form getTask and task - as the plural query.

+1 on this solution.

find Object [s,ss,sh] etc. Having plurals brings confusion to the spec - especially when developers dealing with models that might not be easily pluralized.

This is the main concern I had with pluralising.

English language as many others have plural versions.

Wait till you hear the different varieties there are in French language.

wtrocki commented 4 years ago

I was talking @ralfhandl the other day about how OData deals with this and there is an explicit definition of the model - model can be singleton or array - In case of the graphql crud we assume that everything is array and there are no singletons, but the benefit of splitting model definition and it's the actual name in query, update thing can be beneficial.

This is also what @danielrearden mentioned to me - names should be controllable - so we do not have any artificial variants that differ only by name.

However I find it hard to reflect plural in SDL. Would something like this make sense:

type User @instance(name: "Users") {
}

Seems overkill to me

wtrocki commented 4 years ago

Another crazy idea. Instead of dealing with names we can specify operations and mappings - and even build specific implementation for the language of the choice. This will mean that spec itself will abstract from the name - what you will get is just crud operations with certain capabilities and names itself would be coming from individual implementation.

This will allow us to define mappings to various different solutions - we can map (and transform) from Prisma, Hasura, AppSync in seconds - or throw errors if someone using capability that other platform is not using.

craicoverflow commented 4 years ago

I don't see this as an issue with GraphQLCRUD. I see this as an issue with how it is implemented in Graphback.

"""
@model(plural: "Utilisateurs")
"""

Removing all pluralization will become confusing for users of the API:

findTask(...)
getTask(...)

If above were to happen, it is not obvious what the difference between both queries are.

Pluralization is very important in identifying the action of a resolver/method. If everything is singular the schema API becomes less clear.

wtrocki commented 4 years ago

@craicoverflow but following your comment. How do you see us to remove ambiguity in spec? We are building now forms solution that works with GraphQL CRUD. How we can build queries that will be compatible without using Graphback. Do you see Graphback being used as source for generating schema everywhere?

danielrearden commented 4 years ago

FWIW, some APIs (like Hasura's) don't bother with the distinction between singular and plural at all.

If you did drop the pluralization, something like listTask vs taskById might be sufficiently clear. Just my two cents.

danielrearden commented 4 years ago

WRT Graphback specifically, you could add a plugin config option for an operation name template map with some reasonable defaults

operationNames:
  findById: '${MODEL}ById'
  findAll: 'find${MODEL}'
  # create, update, etc.
wtrocki commented 4 years ago

I think we want to go above graphback here. Graphback (or more specifically GraphQL-serve) is a tool that allows people to print out compatible schema, but if you as the creator will need to adjust your own tool to be compatible just by names that will be bummer - Introducing breaking changes to users.

I will check how we can do this mapping - in both documentation and tooling and see if that is doable. We can imagine if we get the tooling we can map features available in some of the providers and have some translation engine.

My use case is simple - I'm building tons of UI/Client side tools and want them to work. Currently, they work with Graphback - but that is not what we want. If someone has hasura and AppSync backend things should work for those people as well.

This means that my tools can supply different versions of the queries to different providers, but if graphql crud will deal with this then it could be much easier for people to adopt it and it will bring a lots of value.

wtrocki commented 4 years ago

WRT Graphback specifically, you could add a plugin config option for an operation name template map with some reasonable defaults

Great idea!

We need here short term plan and long term.

For the short term, I propose non pluralized names.

For the long term, we will introduce the concept of features and how they are composed in queries which will allow us to map any names used for queries in implementations like AppSync, Hasura, Prisma etc.

Any opinions?

craicoverflow commented 4 years ago

For the short term, I propose non pluralized names.

For the short term this would mean completely changing the semantic structure of the query names so that they make sense.

Simply dropping plurality from the existing names mean that they will single v plural queries become indistinguishable, eg: getTask v findTask.

So as @danielrearden suggested a way to distinguish these is to use something like taskList over findTasks.

machi1990 commented 4 years ago

WRT Graphback specifically, you could add a plugin config option for an operation name template map with some reasonable defaults

Great idea!

We need here short term plan and long term.

For the short term, I propose non pluralized names.

For the long term, we will introduce the concept of features and how they are composed in queries which will allow us to map any names used for queries in implementations like AppSync, Hasura, Prisma etc.

Any opinions?

The short term solution should be easy to map and possibly the default one once the long term solution is there.

To avoid ambiguities of the queries, I second what @craicoverflow @danielrearden proposed, instead of findTasks, we can have listTask.

craicoverflow commented 3 years ago

After today's call, taskList seems to fit best. It makes sense from a grammatical point of view - it is never going to change depending on the word used (Person/People).

wtrocki commented 3 years ago

Perfect! This change would be really beneficial.