Pure GraphQL-compliant version about crystal HOT 14 CLOSED

@meglio nah, use id like normal in your PostgreSQL schema. Just know that PostGraphQL will rename your id column to rowId as a special case.

from crystal.

What do you think that would look like? I’m super open to ideas, but at the moment I don’t necessarily think of it as “Relay GraphQL,” but rather idiomatic GraphQL. The Relay specs are pretty good and well thought out. I feel like Node is a good practice for rows, and I feel like having Connections is best practice for selecting from tables (because we get cursors and pageInfo). Adding fields to support Relay doesn’t hurt non-Relay users and vice-versa. That’s one of the beauties of GraphQL.

The one thing I would like when dropping Relay support is not having to rename id to rowId.

from crystal.

I feel there's an impedance mismatch between the GraphQL spec/docs and PostGraphQL.

IMHO nodes introduces a level of obfuscation. If I have a Posts type, then I expect I can query for posts (which I can for a single record but only if I provide an ID, but not an ID as I know the ID). I don't expect I have to query postsNodes to return rows, or that those rows will returned in a nodes key (although given I need to include the nodes key in my query, it makes sense that it is returned). I'm not convinced nodes are good practice, what's the argument for them?

Global Object Identification is definitely only part of the Relay spec, and definitely muddies the waters.

Connections is cool, I like it, and I like the free cursor-based pagination; however I don't necessarily want it. Given the schema can be introspected, I might want to only expose what GraphQL provides, not what Relay provides, or I might want to do it differently. If PostGraphQL provided GraphQL-compliant-only functionality up front, then I would have that choice.

I'm super excited about this project, and I applaud the effort you have put into it, it really is amazing. I agree that Relay is well thought out. I also agree with all of your arguments about No Lock In and Schema-driven APIs, but I don't feel that's being achieved. If I write my software against PostGraphQL's idiomatic GraphQL, I am locked in; I can't switch to a reference implementation of GraphQL. Although I don't necessarily dislike these additions, I do feel they should be additions.

P.S. As a side issue, rowId should probably return a description when fields for a type is queried.

from crystal.

I feel there's an impedance mismatch between the GraphQL spec/docs and PostGraphQL.

Part of what confuses me about this is that Relay additions are pure GraphQL. The Relay additions are simply a way to structure the GraphQL API. The GraphQL spec is amazing in this way. It can be used to create any API. One of my favorite examples of this is gdom a GraphQL HTML parsing API. So there's no mismatch between GraphQL/PostGraphQL. Only an opinionated GraphQL structure (which is kinda unavoidable). At least our opinions are the current GraphQL idioms and are easily and quickly translated to other GraphQL frameworks.

IMHO nodes introduces a level of obfuscation.

I agree, and nodes is actually the easy way out—it was added for non-Relay users 😊

This pattern was taken from the SWAPI GraphQL API as it was one of the first GraphQL implementations. Except where we have postNodes->nodes, they would have allPosts->posts. The reason I chose the nodes naming convention was because I didn't want to use a pluralization library because that may isolate non-English speakers. Was this the correct decision given PostGraphQL runs in Node.js and can be easily extended for internationalization purposes by users?

I feel like much of this issue all boils down to a distaste for the nodes naming convention. And I can see why. Would you agree with this? Because if we can agree connections are the right abstraction for paginating a list and we can agree the nodes convention is undesirable, then that's a huge piece of Relay convention we've stepped around.

I don't want to alienate users because of Relay support, but I also don't see a use for a field which only returns a list with no connection as pagination is wanted like 90% of the time.

I also don't want to remove global object identification, because that's how almost every GraphQL framework is able to normalize data and build a proper cache.

So what do you think about the compromise of adding a pluralization library to get allPosts->posts? What else would you like to see change?

from crystal.

As a non-relay-user and relatively new PostGraphQL user (and contributor, thanks to @calebmer thoughtful reviews and time), I can relate to @JurgenJocubeit point of view: the relay opinionated decisions get in the way of getting started with the awesomeness of PostGraphQL.

Having a Relay-compatible API available is great, as I understand that Relay is a package of GraphQL API best practices from the trenches but it adds a non trivial amount of complexity that might not be necessary for everyone.

The main problem I had while getting started with PostGraphQL was getting around the id / rowId thing. I did expect the API to reflect my database schema and I did not expect my primary key field to have a different name from what I defined in the table. Having a globally unique id is great but I expected it to have a custom name like _id or anything that isn't defined as a column in my tables.

I also don't want to remove global object identification, because that's how almost every GraphQL framework is able to normalize data and build a proper cache.

From what I understand, the global object id is a Relay addition so that shouldn't be the case if those frameworks support non-Relay GraphQL APIs.

The reason I chose the nodes naming convention was because I didn't want to use a pluralization library because that may isolate non-English speakers

IMHO nodes and postNodes don't get in the way at all, I mind-mapped the node concept to a relational db tuple and it works fine for me until now. What is in my way are the edge and viewer concepts that I still can't mind-map to my simple SQL world. (It is also worth noting that GrapiQL really helps here)

It's not that I can't get those concept, it's just that I identify those as being unnecessary complexity for users that just want a quick and straightforward GraphQL api from their db.

Connections is cool, I like it, and I like the free cursor-based pagination; however I don't necessarily want it.

Agreed, we all know that pagination is hard and that offset + limit pagination is flawed but it's also a concept that is good enough for the majority of use cases while being simpler to use. Having both available would be great.

Regarding PostGraphQL, my opinion is that we should run a "non-Relay" mode by default that maps SQL to GraphQL in a simple way to have a smooth learning curve and add --relay option that would enable all Relay features. Documentation would also be added to introduce Relay concepts for the SQL-aware developer.

from crystal.

Ok, so could we get a list of things we would want from a --no-relay CLI argument? I would prefer --no-relay to avoid breaking changes, in a major version bump this may be changed however.

After that, we can assess complexity and look at how to integrate with the current codebase.

from crystal.

This might be an interesting read for participants in this thread: facebook/relay#1061

Specifically this comment: facebook/relay#1061 (comment)

GraphQL is planning to standardize the global identification pattern by using an __id field. This is very exciting news for PostGraphQL!

Just another note, before anyone does work on this issue, they should share their full plan. If at all possible I would prefer finding a way to marry the two styles into one API instead of fragmenting it.

from crystal.

Awesome work on the __id, field can't wait to land this feature!

I put some thought into the target design and I've come to the conclusion that a --no-relay option is not that good, it's just of no value. We are better offering various ways of doing the same thing, one relay-compliant and one simple and easy to get started with.

As a first step I suggest that we add limit + offset pagination as an alternative to cursors
Something like this:

query {
  projectNodes (orderBy: TITLE, offset: 30, limit: 10) {
    nodes {
      id
    }
  }
}

Or an alternative that does not leak the underlying SQL storage abstraction:

query {
  projectNodes (orderBy: TITLE, page: 3, page_size: 10) {
    nodes {
      id
    }
  }
}

from crystal.

Currently, we do offer limit/offset pagination using the arguments first + offset. Is that enough? Should we also have an alias for first, limit?

from crystal.

Oh, I did not know that.

I would then add an example in the README to point non Relay users and new GraphQL users the direction they should look at.

Something like this:

query {
  postNodes (
      authorId: 12, 
      orderBy: HEADLINE, 
      descending: true, 
      first: 10, 
      offset: 20
  ) {
    nodes {
      headline
      body
      author: userByAuthorId {
        name
      }
    }
  }
}

from crystal.

Reading the GraphQL id struggles you've been fighting since April gives me heart. It's an interesting thread and your concerns are very closely aligned with my thoughts. I probably just didn't know how to express them. Bravo for getting Facebook to consider __id.

Just a side thought, would it be possible to create an extension like uuid-ossp that builds a relay-compliant global id? And would that help at all? This would mean we could use id as the database table row identifiers natively, and support relay without the need for a rowId or equivalent.

As another side note, I really like the anonymity uuids give my records. Record IDs are not easily guessable.

from crystal.

@JurgenJocubeit my favorite solution to this problem is the __id proposal. The difficulty with using uuids or a similar format straight up is that while they’d be globally unique, they do not give us information about the table the row is stored in. The current format, a base64 encoded version of user:2, is advantageous because not only does it guarantee global uniqueness, but it also tells us that the node with this id is of the user table. 531bc28b-2f69-4d56-9f1c-e84d03c268b7 alone would not give us this information, but user:531bc28b-2f69-4d56-9f1c-e84d03c268b7 would.

If a PostgreSQL UUID extension has the ability to select a single row and give us type information (table) then we could adopt that approach.

from crystal.

Just to make it clear for a beginner like me, when designing a new database to be used with your library, is it recommended to use rowId column primary keys to make it Relay compatible?

from crystal.

I’m going to close this as I feel like much of the ‘features’ that were obtrusive for non-Relay users have been either removed or reworked.

• By default ids no longer rename to rowId. We still have global ids, they are just called __id.
• Connection fields use the plural name of the type and not “nodes.” For example, allPosts vs. postNodes.

If there’s anything else that exists to support Relay that you feel is too obstrusive, let me know! I definitely don’t want to alienate non-Relay users especially now that Relay 2 will probably make a lot of the Relay 1 specifications irrelevant 😉

from crystal.