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

More directives for updating the store declaratively #3123

Closed
zth opened this issue Jul 13, 2020 · 11 comments
Closed

More directives for updating the store declaratively #3123

zth opened this issue Jul 13, 2020 · 11 comments

Comments

@zth
Copy link
Contributor

zth commented Jul 13, 2020

Building on the recent additions of @deleteRecord, @prependEdge and @appendEdge, here's a few ideas for more directives. I'd be happy to take a stab at implementing these if they seem like good ideas.

Adding optional connections arg to @deleteRecord

This would add an optional arg to @deleteRecord, like @deleteRecord(connections: [String!]). The effect of using connections would be that in addition to deleting the node from the store, the node + its edge would also be deleted from the connections provided.

Alternative: A dedicated directive

An alternative to the above would be to add another directive specifically for deleting any edge associated with the record ID from any of the supplied connections. Maybe @deleteNodeEdgeFromConnections(connections: [String!]!).

Adding @deleteFromList(parentID: ID!, fieldName: String!)

This directive would delete a node from a list, as in filter it out from the linked records of a field on a specific parent record. This covers only the basic use cases (no way to do multiple lists, use arguments, etc), and it's not quite as nice as the connection directives since there's no way to get an __id on a list, right?

Adding @appendNodeToList(parentID: ID!, fieldName: String!) and @prependNodeToList(parentID: ID!, fieldName: String!)

Doing the same thing as @prependEdge and @appendEdge but for lists. Same caveats as @deleteFromList, namely that this would only cover the basic use cases.

Adding@createAndAppendEdge + @createAndPrependEdge

Same as @prependEdge/@appendEdge, but instead of taking an actual edge returned from the server, this would just take a returned node, create a new edge for that node, and then insert it into the connections. A lot of APIs don't return an actual edge (just a created entity of the thing you're creating), and this could simplify working with those.

Adding @linkTo(parentID: ID!, fieldName: String!)

This would basically do a store.get(parentID).setLinkedRecord(createdRecordId, fieldName). Just linking records together. Not sure if this really is valuable without a way of providing filters/arguments though.

Adding plural versions of all directives where it makes sense

@prependEdges and @appendEdges, as examples of the concept, would do what their singular counter parts do, but for a list of edges rather than just a single one.

The same logic can be applied to @deleteRecords and most of the directives proposed above.


Just a few ideas! Happy to discuss them, and I'll add more when I think of them.

@enisdenjo
Copy link
Contributor

Awesome suggestions, this would be pretty rad to have! 👀

Two more ideas:

Adding @appendToConnectionTypes(connectionTypeName: String!)

Where the connectionType is a valid GraphQL Cursor Connection Type. Straight from the spec:

Any type whose name ends in “Connection” is considered by this spec to be a Connection Type. Connection types must be an “Object” as defined in the “Type System” section of the GraphQL Specification.

Don't know exactly how the imperative store operations would look like, but in essence it would append the node/edge to every connection of the passed type.

Adding @deleteFromConnectionTypes(connectionTypeName: String!)

Simply the @appendToConnectionTypes directive counterpart.

@sibelius
Copy link
Contributor

I think we need a way to make it easy to add this custom directive in user land

each project has different use cases that we can't cover in the relay core code

@josephsavona
Copy link
Contributor

I think we need a way to make it easy to add this custom directive in user land

This is not something we intend to support. GraphQL is a DSL, and DSLs aren't turing-complete and can never address every possible use-case. We would like to hold that line at a small number of declarative mutations that we expect to be useful in every conceivable project (basic CRUD). For advanced operations developers can use updaters as before.

@josephsavona
Copy link
Contributor

josephsavona commented Jul 16, 2020

With the criteria I stated above ("small number of declarative mutations that we expect to be useful in every conceivable project (basic CRUD)") in mind, of the directives proposed here I'm inclined to include only the following:

  • appendNode/prependNode alternatives to the existing appendEdge/prependEdge directives
  • directive for deleting an edge/node from a connection
  • support deleting/appending/prepending multiple items (possibly by making all the existing directives just work on plurals too)

@zth
Copy link
Contributor Author

zth commented Jul 16, 2020

directive for deleting an edge/node from a connection

@josephsavona So this should be a distinct directive and not something piggy backing on @deleteRecord, is that correctly understood?

@josephsavona
Copy link
Contributor

I don't immediately see a clear reason why either (separate/same directive) is better.

@zth
Copy link
Contributor Author

zth commented Jul 31, 2020

For anyone interested, I've started implementing what I perceived from this discussion as the directives/features that are likely to be accepted:

We'll see if they can be molded into something mergeable. I'll keep this post updated.

@damikun
Copy link

damikun commented Aug 12, 2020

@zth Hi first super idea about this directives... Is any of @deleteRecord or @apendNode or @prependNode available in some experimental version of relay ?

@zth
Copy link
Contributor Author

zth commented Aug 13, 2020

Hey @damikun ! @deleteRecord is part of the initial directives added to Relay and is available in ^10.0.0. The next release will add support for using @deleteRecord on lists of ID too. @appendNode and @prependNode + $connections support for @deleteRecord are currently waiting for review.

@zth
Copy link
Contributor Author

zth commented Sep 5, 2020

Update:

#3135 plural support for @deleteRecord has landed 🎉
#3155 @appendNode and @prependNode has landed 🎉
PR for @deleteEdge opened here: #3177

@zth
Copy link
Contributor Author

zth commented Sep 30, 2020

#3177 has now landed. That concludes the implementation of the list of store directives we arrived at in this thread, so I'll go ahead and close this thread.

@zth zth closed this as completed Sep 30, 2020
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

5 participants