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

Putting data on edges (or not) #52

Open
eapache-opslevel opened this issue Sep 5, 2023 · 3 comments
Open

Putting data on edges (or not) #52

eapache-opslevel opened this issue Sep 5, 2023 · 3 comments

Comments

@eapache-opslevel
Copy link

eapache-opslevel commented Sep 5, 2023

This is a design topic which has been discussed a bunch internally at my current company, and which is not addressed in the design tutorial. IIRC at Shopify we ended up at (what I believe to be) the correct answer without much discussion and never really thought about it, but in hindsight I think that was a fluke, and it's worth making an official design ruling on. This should probably be worked into the design tutorial, or tacked on as another appendix or something.

My current belief is:

While there are some cases where putting data directly on connection edge types is OK, there are many where it ends up causing issues, and should be avoided; it is best to adopt the simple rule never to put data on edge types. This may result in a few extra types in the schema, but simplifies reading/interpretation because it becomes safe to assume that edge types are always just boilerplate and can be safely ignored.

The cases where putting data on edges can cause issues include:

  • When you have a many-to-many relation, with connections running both ways from A to B and also from B to A. Putting data directly on the edge would require you to duplicate fields between the A-to-B-edge type and the B-to-A-edge type.
  • When you have several types (A, B, C), all with one-to-many relationships to the same type (X). This is solvable, but you would have to remember to create/use AXEdge and BXEdge etc. instead of the standard/default generated XEdge, which can't reasonably have edge-data for two or more different relationships.
  • When you have a mutation operating on the relationship between an A and a B, in which case that mutation would have to duplicate fields from the edge type, or return an edge type directly (in addition to whichever of A or B isn't covered by the edge), which is gross.
  • Probably others I'm not thinking of right now.
@swalkinshaw
Copy link
Contributor

but in hindsight I think that was a fluke,

I think that's correct. Also I just verified that Shopify's schema still has no edge types with additional fields as a confirmation.

This makes sense to capture and I'm in favour as well if you want to contribute it

@tgwizard
Copy link
Member

If edges are boilerplate and can just be ignored, then why do we have them? There is the nodes field, which is equivalent to edges.map(&:node). Is that enough?

One field that's generally put on edges is the cursor, to start a pagination from that specific edge, rather than the end of the current page. Not sure how much we use that, but would that be still be useful?


In the scenario where data is actually naturally stored on the edge (e.g. in a friend relationship between two users, with status and timestamps etc on the edge), then, without the data on edge, we'd need a UserFriend type to capture that data. If the nodes aren't of the same type (e.g. a User has a relationship with a Shop), then we'd need a UserShop and ShopUser type, and remember to create/use those?

@eapache-opslevel
Copy link
Author

Yeah, the boilerplate edge types still need to exist in order to hold the cursor field which is useful in various situations. cursor is part of the pagination spec, so I wasn't including it in "data on an edge", I was thinking of it as part of the boilerplate. The main conversations I've had about this offline have been focused on whether or not to put business data (e.g. friendship status, as in your example) on edges.


In the scenario where data is naturally stored on the edge, then yes, you do need to manually create a GraphQL type for each edge or "join table" or however you think about it. But these types can be bidirectional at least (e.g. you don't need both UserShop and ShopUser, you just need one or the other and it can be used for both cases since the data on it is the same either way).

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

3 participants