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

Relay mutation types support #43

Closed
msimulcik opened this issue May 8, 2016 · 11 comments
Closed

Relay mutation types support #43

msimulcik opened this issue May 8, 2016 · 11 comments

Comments

@msimulcik
Copy link

Hi, I'm trying to write Relay client backed by postgrqphql, but I'm having trouble with RANGE_ADD mutation type, which I want to use for adding a new person to the list of persons.

InsertPersonPayload type only contains person: Person and clientMutationId: String fields, but according to relay documentation this mutation type requires a parent, a connection, and the name of the newly created edge from the payload to be specified.

I'm new to GraphQL and Relay and maybe I'm missing something here, but it seems to me that with current mutation payloads it is not possible to define all Relay mutation types. I checked how Reindex solves this and I think we need to add a field containing newly inserted edge (personEdge) and a viewer field that will act as a single entry node allowing us to access connections (like personNodes) that are currently available only in Query type. The single entry node field was already mentioned in #38 but in the context of queries.

@calebmer
Copy link
Collaborator

calebmer commented May 9, 2016

Could you draft out a quick schema of what you'd need? This shouldn't be too hard to add, I just want to see exactly what you have in mind.

Maybe this is related to #38? (/cc @hardchor)

@msimulcik
Copy link
Author

Sure, I'm going to put something together after work. In the meantime you can check doc pages on mutations and queries from Reindex project, which is very similar to postgraphql. While Postgraphql generates the GraphQL schema (queries and mutations) based on the DB schema, Reindex generates it based on a custom JSON schema, that describes entities and relations. I think it could be a good inspiration for postgrqaphql as they have already solved these issues with the structure of query and mutations payload types.

@msimulcik
Copy link
Author

msimulcik commented May 9, 2016

I will demonstrate the problem on simple todo list. First create a todo table with few rows

create table todo(todo_id serial primary key, todo_text varchar(255), complete boolean);
insert into todo(todo_text, complete) values ('foo', false), ('bar', false);

Now I want to list all todos in Relay

// App.js
class App extends React.Component {
  render() {
    return (
      <div>
        <h1>Todo list</h1>
        <ul>
          {this.props.todos.edges.map(edge =>
            <li key={edge.node.id}>{edge.node.todoText}</li>
          )}
        </ul>
      </div>
    );
  }
}

export default Relay.createContainer(App, {
  fragments: {
    todos: () => Relay.QL`
      fragment on TodoConnection {
        edges {
          node {
            id
            todoText,
            complete
          }
        }
      }
    `,
  },
});

// AppHomeRoute.js
export default class extends Relay.Route {
  static queries = {
    todos: () => Relay.QL`
      query {
        todoNodes(orderBy: TODO_ID)
      }
    `,
  };
  static routeName = 'AppHomeRoute';
}

The problem here is that with connections as root fields it is not possible to have ordering specified in the fragment for App component. This is related to #38. By moving all connections under single root field, lets call it viewer, we could write

// App.js
class App extends React.Component {
  render() {
    return (
      <div>
        <h1>Todo list</h1>
        <ul>
          {this.props.viewer.todoNodes.edges.map(edge =>
            <li key={edge.node.id}>{edge.node.todoText}</li>
          )}
        </ul>
      </div>
    );
  }
}

export default Relay.createContainer(App, {
  fragments: {
    viewer: () => Relay.QL`
      fragment on ViewerType {
        todoNodes(orderBy: TODO_ID) {
          edges {
            node {
              id
              todoText,
              complete
            }
          }  
        }
      }
    `,
  },
});

// AppHomeRoute.js
export default class extends Relay.Route {
  static queries = {
    viewer: () => Relay.QL`
      query {
        viewer
      }
    `,
  };
  static routeName = 'AppHomeRoute';
}

Now I want to create new todo with insertTodo mutation provided by postgraphql. And here comes the second problem. I need to attach newly created todo edge to todoConnection. Relay provides RANGE_ADD mutation type for this operation, but InsertTodoPayload does not provide enough data for its configuration. In addition to newly inserted node, mutation payload must also include viewer field with all connections and newly inserted edge. Then we could write

export default class AddTodoMutation extends Relay.Mutation {

...

getFatQuery() {
  return Relay.QL`
    fragment on InsertTodoPayload {
      todoEdge,
      viewer {
        todoNodes
      }
    }
  `;
}

...

getConfigs() {
  return [{
    type: 'RANGE_ADD',
    parentName: 'viewer',
    parentID: this.props.viewer.id,
    connectionName: 'todoNodes',
    edgeName: 'todoEdge',
    rangeBehaviors: {
      '': 'prepend',
    },
  }];
}

...
}

I hope this example is sufficient to understand these Relay related problems. I think that if we want postgraphql to be easily integrated with Relay, we have to implement not only Relay specifications for GraphQL, but also things not directly specified, but required for basic use cases. If we agree on details, I will be glad to help you with implementation. This shouldn't be hard.

@calebmer
Copy link
Collaborator

See #49, is this all you really need?

@calebmer
Copy link
Collaborator

calebmer commented Jun 1, 2016

The basic viewer field was released in 1.5.0, hopefully we can get a PR implementing the edge field soon.

@msimulcik we implement cursors using an orderBy argument, so cursors may change if a different value for orderBy is used. How do we decide which orderBy value to use for a todoEdge field?

@msimulcik
Copy link
Author

Maybe we could consider using the ID column for cursor implementation as suggested in this comment from the Relay repo

@ferdinandsalis
Copy link
Contributor

I have started working on this. See this commit https://github.com/ferdinandsalis/postgraphql/commit/4dda79a058e0a1b67f880ae6b1be1ffdcaffeeda.

How do we decide which orderBy value to use for a todoEdge field?

I followed the lead of this stack overflow answer — https://stackoverflow.com/questions/32613879/should-the-opaque-cursors-in-connections-be-stable-across-different-field-args

@msimulcik
Copy link
Author

@ferdinandsalis, what is the status of your work on mutation edges? I took a brief look at your code and it seems ok to me. Is there something I can help you with so we can open PR?

@ferdinandsalis
Copy link
Contributor

@msimulcik I have opened a PR — #68. Really appreciate feedback. Thanks!

@ferdinandsalis
Copy link
Contributor

This just landed in master — See #68

@calebmer
Copy link
Collaborator

Released in 1.6.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants