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

Support Root Fields w/o Node Mapping #112

Closed
yungsters opened this Issue Aug 18, 2015 · 51 comments

Comments

Projects
None yet
@yungsters
Contributor

yungsters commented Aug 18, 2015

Problem

Relay currently only knows how to handle these three types of root fields:

  • Root field with no arguments and queries a single node.
    • e.g. empire queries {id: "123", ...}
  • Root field with one argument and queries a single node.
    • e.g. ship(id: "456") queries {id: "456", ...}
  • Root field with one array argument and queries an array of nodes.
    • e.g. ships(ids: ["456", "789"]) queries [{id: "456", ...}, {id: "789", ...}]

However, it has become clear that Relay needs to support any kind of root field. For example:

  • Root field with multiple arguments.
  • Root field with no arguments and queries an array of nodes.
  • Root field that is a connection.

Workaround

For now, the unsupported use cases can be implemented by creating a "global node", commonly called the viewer. You can then add arbitrary fields to viewer.

static fragments = {
  viewer: () => Relay.QL`
    fragment on Viewer {
      # Can return an array of users.
      users
    }
  `
};

Rationale

Historically, GraphQL (as used internally at Facebook) did not have a root type or root fields. Instead, it had special "root calls" such as node, nodes, me, and usernames. Much of Relay was built on top of this assumption that the "root calls" return nodes.

For example, when we fetch me and get {id: "123", ...}, we record the association between the me root field and the node with ID of 123. Now, if we ever encounter another query for me, we can check our store for the node with ID of 123 and resolve the query without having to potentially re-fetch all of the fields we already have for me.

Another example, when we fetch nodes(ids: ["123", "456"]), we record the association between each argument and their respective response nodes. This allows us to fulfill queries for node(id: "123") and node(id: "456") even though we may never have independently queried for either before. (We would also be able to fulfill me if the association from above was established.)

Next Steps

  • Support literal enum and input object values (#894)
  • Support arbitrary values in root calls (#895)
  • Define a consistent method for annotating a root argument as "identifying". Identifying arguments have a 1:1 correspondence between the argument value and the id of the response. Currently all root arguments are assumed to be identifying.
  • Allow arbitrary root calls with or without identifying arguments.
@devknoll

This comment has been minimized.

Show comment
Hide comment
@devknoll

devknoll Aug 25, 2015

Contributor

@yungsters @josephsavona is it possible that this change would also allow us to remove the Relay-specific Input Objects on mutations? It seems super strange that GraphQL supports multiple arguments but then Relay forces them to all be wrapped up ;-)

Edit: would lose the ability to have a single variable in the document and stuff all the values into a variable as an input object though.

Contributor

devknoll commented Aug 25, 2015

@yungsters @josephsavona is it possible that this change would also allow us to remove the Relay-specific Input Objects on mutations? It seems super strange that GraphQL supports multiple arguments but then Relay forces them to all be wrapped up ;-)

Edit: would lose the ability to have a single variable in the document and stuff all the values into a variable as an input object though.

@vincentriemer

This comment has been minimized.

Show comment
Hide comment
@vincentriemer

vincentriemer Sep 16, 2015

Do you have any example code as to how the viewer root field would look like on the server side (nodejs)?

vincentriemer commented Sep 16, 2015

Do you have any example code as to how the viewer root field would look like on the server side (nodejs)?

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Sep 17, 2015

Contributor

@vincentriemer take a look at the todo app's schema for an example of setting up a root viewer field.

Contributor

josephsavona commented Sep 17, 2015

@vincentriemer take a look at the todo app's schema for an example of setting up a root viewer field.

@nickretallack

This comment has been minimized.

Show comment
Hide comment
@nickretallack

nickretallack Oct 9, 2015

This really confused me. This limitation should be spelled out more clearly in the docs and in relay-starter-kit.

There is already a root object: the GraphQLSchema. Its direct descendants can be GraphQLObjectTypes with fields in them that can be other GraphQLObjectTypes. However, it seems that the first level GraphQLObjectTypes are different from all others in that Relay refuses to query them in certain ways.

In the starter kit, the root object is queryType, and it defines a field viewer which is a userType. queryType even has a comment in it: // Add your own root fields here. But you don't want to add your own root fields here because they will have limited functionality. You really want to add them into the viewer. So does it make sense for the viewer to be a userType? I don't think it does.

I think the starter kit should change the userType into a generic wrapper type and move the comment into that type instead.

Btw, I really wanted to create a connection type at the root level. Requiring two levels of GraphQLObjectType in a row to get to normal functionality seems silly.

As for making things continue to work at Facebook, I don't know the specifics, but it seems like removing this restriction shouldn't negatively impact anything, right? You'd just need to make Relay's parser smart enough to parse queries that don't fall under this restriction. If you wanted to keep the restriction internally, you could write some sort of validation process to check if anyone is violating the restriction before submitting their code.

nickretallack commented Oct 9, 2015

This really confused me. This limitation should be spelled out more clearly in the docs and in relay-starter-kit.

There is already a root object: the GraphQLSchema. Its direct descendants can be GraphQLObjectTypes with fields in them that can be other GraphQLObjectTypes. However, it seems that the first level GraphQLObjectTypes are different from all others in that Relay refuses to query them in certain ways.

In the starter kit, the root object is queryType, and it defines a field viewer which is a userType. queryType even has a comment in it: // Add your own root fields here. But you don't want to add your own root fields here because they will have limited functionality. You really want to add them into the viewer. So does it make sense for the viewer to be a userType? I don't think it does.

I think the starter kit should change the userType into a generic wrapper type and move the comment into that type instead.

Btw, I really wanted to create a connection type at the root level. Requiring two levels of GraphQLObjectType in a row to get to normal functionality seems silly.

As for making things continue to work at Facebook, I don't know the specifics, but it seems like removing this restriction shouldn't negatively impact anything, right? You'd just need to make Relay's parser smart enough to parse queries that don't fall under this restriction. If you wanted to keep the restriction internally, you could write some sort of validation process to check if anyone is violating the restriction before submitting their code.

@steveluscher

This comment has been minimized.

Show comment
Hide comment
@steveluscher

steveluscher Oct 9, 2015

Contributor

Thanks for this, @nickretallack. Right now, I'm working on literally nothing else but enabling identifying/non-identifying singular/plural fields at the root, including connections. Hold tight!

Contributor

steveluscher commented Oct 9, 2015

Thanks for this, @nickretallack. Right now, I'm working on literally nothing else but enabling identifying/non-identifying singular/plural fields at the root, including connections. Hold tight!

@RavenHursT

This comment has been minimized.

Show comment
Hide comment
@RavenHursT

RavenHursT commented Oct 27, 2015

👍

ButuzGOL added a commit to slicketty/watchme-backend that referenced this issue Nov 3, 2015

Small updates:
- Update ativities based store visit facebook/relay#112
- Path to schema in cli use npm run update-schema "../../watchme/data/schema.json"

luisescobedo added a commit to openengine/openengine-webpack that referenced this issue Nov 14, 2015

Card creation with user assignment v1 done! That was painful. There i…
…s an open bug on relay that makes retrieving connections at the Root Query very difficult: facebook/relay#112

Currently there is a workaround, it's ugly but it works.
@stevewillard

This comment has been minimized.

Show comment
Hide comment
@stevewillard

stevewillard Nov 16, 2015

Just ran into this. Would love to see root fields with multiple arguments!

stevewillard commented Nov 16, 2015

Just ran into this. Would love to see root fields with multiple arguments!

@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Dec 23, 2015

One problem with the viewer approach is that Relay seems not to let you specify multiple levels of nesting at the root query level:

const AppContainer = Relay.createContainer(App, {
  fragments: {
    projects: () => Relay.QL`
      fragment on Project {
        name
        isMyProject
      }
    `
  }
});


const AppQueries = {
  projects: () => Relay.QL`
    query {
      viewer {
        projects
      }
    }
  `
};

ReactDOM.render(
  <RelayRouter history={history}>
    <Route path="/" component={AppContainer} queries={AppQueries} />
  </RelayRouter>,
  mountNode
);

Is there a workaround for making that work? Or should my root query always just be statically set to query { viewer }?

ForbesLindesay commented Dec 23, 2015

One problem with the viewer approach is that Relay seems not to let you specify multiple levels of nesting at the root query level:

const AppContainer = Relay.createContainer(App, {
  fragments: {
    projects: () => Relay.QL`
      fragment on Project {
        name
        isMyProject
      }
    `
  }
});


const AppQueries = {
  projects: () => Relay.QL`
    query {
      viewer {
        projects
      }
    }
  `
};

ReactDOM.render(
  <RelayRouter history={history}>
    <Route path="/" component={AppContainer} queries={AppQueries} />
  </RelayRouter>,
  mountNode
);

Is there a workaround for making that work? Or should my root query always just be statically set to query { viewer }?

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Dec 23, 2015

Contributor

@ForbesLindesay There's no technical reason we couldn't allow nesting in the root query. The only thing that would have to change is GraphQLFragmentPointer's createForRoot method, which makes a simplifying assumption that the fragment is a direct child - any interest in sending a PR? ;-)

Otherwise, yes, most apps end up with mostly viewer/node root queries.

Contributor

josephsavona commented Dec 23, 2015

@ForbesLindesay There's no technical reason we couldn't allow nesting in the root query. The only thing that would have to change is GraphQLFragmentPointer's createForRoot method, which makes a simplifying assumption that the fragment is a direct child - any interest in sending a PR? ;-)

Otherwise, yes, most apps end up with mostly viewer/node root queries.

@mattecapu

This comment has been minimized.

Show comment
Hide comment
@mattecapu

mattecapu Apr 11, 2016

@josephsavona thank you, can I know why this isn't a priority? I was pretty baffled to discover Relay isn't capable yet to do this kind of things as they seem very natural.
As of @dschafer's comment, it looks like a workaround for the multiple-parameters issue, while I need to fetch an array

Comp1 = Relay.createContainer(Comp1, {
    fragments: {
        comp1: () => Relay.QL`
            fragment on Comp1 @relay(plural: true) {
                ${childComp2.getFragment('comp2')}
            }`
    }
});

export const Comp1Queries = {
    comp1: () => Relay.QL`
        query { comp1s(id: $comp1ID) }`
};

But I get

RelayOSSNodeInterface: Expected payload for root field comp1s to be a single non-array result, instead received an array with 1 results.

And I'm not really grasping the viewer workaround.

mattecapu commented Apr 11, 2016

@josephsavona thank you, can I know why this isn't a priority? I was pretty baffled to discover Relay isn't capable yet to do this kind of things as they seem very natural.
As of @dschafer's comment, it looks like a workaround for the multiple-parameters issue, while I need to fetch an array

Comp1 = Relay.createContainer(Comp1, {
    fragments: {
        comp1: () => Relay.QL`
            fragment on Comp1 @relay(plural: true) {
                ${childComp2.getFragment('comp2')}
            }`
    }
});

export const Comp1Queries = {
    comp1: () => Relay.QL`
        query { comp1s(id: $comp1ID) }`
};

But I get

RelayOSSNodeInterface: Expected payload for root field comp1s to be a single non-array result, instead received an array with 1 results.

And I'm not really grasping the viewer workaround.

@NevilleS

This comment has been minimized.

Show comment
Hide comment
@NevilleS

NevilleS Apr 11, 2016

Contributor

FWIW, since the workaround is so simple (add a "viewer"-esque field on your
root query), I'm pretty OK that the Relay team is focusing on other stuff 😁

On Mon, Apr 11, 2016, 10:35 AM Matteo Capucci notifications@github.com
wrote:

@josephsavona https://github.com/josephsavona thank you, can I know why
this isn't a priority? I was pretty baffled to discover Relay isn't capable
yet to do this kind of things as they seem very natural.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#112 (comment)

Contributor

NevilleS commented Apr 11, 2016

FWIW, since the workaround is so simple (add a "viewer"-esque field on your
root query), I'm pretty OK that the Relay team is focusing on other stuff 😁

On Mon, Apr 11, 2016, 10:35 AM Matteo Capucci notifications@github.com
wrote:

@josephsavona https://github.com/josephsavona thank you, can I know why
this isn't a priority? I was pretty baffled to discover Relay isn't capable
yet to do this kind of things as they seem very natural.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#112 (comment)

@wincent

This comment has been minimized.

Show comment
Hide comment
@wincent

wincent Apr 11, 2016

Contributor

@mattecapu:

can I know why this isn't a priority?

Prioritization is tough, because there are always a lot more things that we would like to do than we have immediate capacity for. Even articulating why decisions are made the way they are can be tough. Perhaps the best way to get insight into what we're working on and why is to take a look at our meeting notes. Let us know if you'd like to know more about any of the things that we're actively working on mentioned in there.

Contributor

wincent commented Apr 11, 2016

@mattecapu:

can I know why this isn't a priority?

Prioritization is tough, because there are always a lot more things that we would like to do than we have immediate capacity for. Even articulating why decisions are made the way they are can be tough. Perhaps the best way to get insight into what we're working on and why is to take a look at our meeting notes. Let us know if you'd like to know more about any of the things that we're actively working on mentioned in there.

@mattecapu

This comment has been minimized.

Show comment
Hide comment
@mattecapu

mattecapu Apr 11, 2016

@NevilleS the issue is that this thread is perhaps the only documentation of this "feature", so it's a bit disappointing to dive into Relay and then stumble upon this obstacle. Moreover the viewer design pattern is not explained anywhere (at least I can't find an explanation, I'll be happy to be wrong on this!) and as a beginner I scratched my head looking at the example and thinking "Why this?".
In fact I'd be happy to understand how to work around this for now.

@wincent I may sound pedant/unrespectful but actually I'm a great fan of the whole React/Relay/GraphQL thing and I understand the last two are still a pretty novel technology so it's fair to have some issues! I want you (and the rest of the team) to know I really appreciate your work and effort in doing this! OSS is hard and you're doing fine anyway 😄
The question was more a matter of understanding, because the queries I wrote seemed straightforward to me and clashing with this issue made me wonder if I really understood how Relay is supposed to work or I'm missing something.

mattecapu commented Apr 11, 2016

@NevilleS the issue is that this thread is perhaps the only documentation of this "feature", so it's a bit disappointing to dive into Relay and then stumble upon this obstacle. Moreover the viewer design pattern is not explained anywhere (at least I can't find an explanation, I'll be happy to be wrong on this!) and as a beginner I scratched my head looking at the example and thinking "Why this?".
In fact I'd be happy to understand how to work around this for now.

@wincent I may sound pedant/unrespectful but actually I'm a great fan of the whole React/Relay/GraphQL thing and I understand the last two are still a pretty novel technology so it's fair to have some issues! I want you (and the rest of the team) to know I really appreciate your work and effort in doing this! OSS is hard and you're doing fine anyway 😄
The question was more a matter of understanding, because the queries I wrote seemed straightforward to me and clashing with this issue made me wonder if I really understood how Relay is supposed to work or I'm missing something.

@mattecapu

This comment has been minimized.

Show comment
Hide comment
@mattecapu

mattecapu Apr 12, 2016

Okay building on @dschafer's solution I managed to work around this. For now is just an ad-hock hack but I hope to encapsulate this logic in a general-purpose wrapper.

I edited the code above in the following way:

const RootQuery = new GraphQLObjectType({
    name: 'RootQuery',
    description: 'Root query',
    fields: () => ({
        // HACK to make queries at root level
        rootHack: {
            type: new GraphQLNonNull(RootQuery),
            resolve: () => ({})
        },
        ...restOfRootType
});
// ...
export default new GraphQLSchema({
    query: RootQuery
});
class Comp1 extends React.Component {
    render() {
        // comp1s, in my case, is an array (its type is GraphQLList(comp1))
        return (
             <Comp2 comp1={this.props.comp1query.comp1s} />
        );
    }
}

Comp1 = Relay.createContainer(Comp1, {
    initialVariables: {
        // this is then changed by react-router based on a URL parameter with the same name
        comp1ID: 0
    },
    fragments: {
        comp1query: () => Relay.QL`
            fragment on RootQuery {
                comp1s(id: $comp1ID) {
                    ${childComp2.getFragment('comp2')}
                }
            }`
    }
});

// this will be the same for every page
const Comp1Queries = {
    comp1: () => Relay.QL`
        query { rootHack }`
};

My react-router routes

<Router>
        <Route path={"/comp1/:comp1ID"}
            component={comp1} queries={Comp1Queries}
            prepareParams={({ comp1ID }) => ({ comp1ID: parseInt(comp1ID) })} />
</Router>

Hope to help someone with this

mattecapu commented Apr 12, 2016

Okay building on @dschafer's solution I managed to work around this. For now is just an ad-hock hack but I hope to encapsulate this logic in a general-purpose wrapper.

I edited the code above in the following way:

const RootQuery = new GraphQLObjectType({
    name: 'RootQuery',
    description: 'Root query',
    fields: () => ({
        // HACK to make queries at root level
        rootHack: {
            type: new GraphQLNonNull(RootQuery),
            resolve: () => ({})
        },
        ...restOfRootType
});
// ...
export default new GraphQLSchema({
    query: RootQuery
});
class Comp1 extends React.Component {
    render() {
        // comp1s, in my case, is an array (its type is GraphQLList(comp1))
        return (
             <Comp2 comp1={this.props.comp1query.comp1s} />
        );
    }
}

Comp1 = Relay.createContainer(Comp1, {
    initialVariables: {
        // this is then changed by react-router based on a URL parameter with the same name
        comp1ID: 0
    },
    fragments: {
        comp1query: () => Relay.QL`
            fragment on RootQuery {
                comp1s(id: $comp1ID) {
                    ${childComp2.getFragment('comp2')}
                }
            }`
    }
});

// this will be the same for every page
const Comp1Queries = {
    comp1: () => Relay.QL`
        query { rootHack }`
};

My react-router routes

<Router>
        <Route path={"/comp1/:comp1ID"}
            component={comp1} queries={Comp1Queries}
            prepareParams={({ comp1ID }) => ({ comp1ID: parseInt(comp1ID) })} />
</Router>

Hope to help someone with this

@jardakotesovec

This comment has been minimized.

Show comment
Hide comment
@jardakotesovec

jardakotesovec May 4, 2016

Contributor

Do I understand correctly that this issues covers just root fields? I was wondering if there are any existing or planned options how to tell relay that some argument is identifying or that its the graphQL id. Therefore Relay could just fetch additional data.

It would make fetching some additional information very convenient, because if I want get (on demand) some additional information in some component I need to do it in fragment. I don't see how I could make it with root fields which are always defined along with root container&component.

Contributor

jardakotesovec commented May 4, 2016

Do I understand correctly that this issues covers just root fields? I was wondering if there are any existing or planned options how to tell relay that some argument is identifying or that its the graphQL id. Therefore Relay could just fetch additional data.

It would make fetching some additional information very convenient, because if I want get (on demand) some additional information in some component I need to do it in fragment. I don't see how I could make it with root fields which are always defined along with root container&component.

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona May 4, 2016

Contributor

any existing or planned options how to tell relay that some argument is identifying or that its the graphQL id

Can you clarify how this would be different than node?

It would make fetching some additional information very convenient,

Hmm. Can you clarify your goal here? There are two options for fetching data in a component, whichever is more convenient: use setVariables/forceFetch or manually construct a query and fetch it. The second option is easy to overlook, but you can always create a root query without a route and fetch it:

const query = Relay.createQuery(Relay.QL`query Foo { ... }`, variables); 
RelayStore.primeCache({query}, readyState => {
  if (readyState.error) {
    console.error(readyState.error);
  } else if (readyState.done) {
    // data is ready
  }
});

Note that this method will write the results of the query into the cache, and all affected components will re-render if necessary (i.e. you don't need to call setState or anything in the callback above).

Contributor

josephsavona commented May 4, 2016

any existing or planned options how to tell relay that some argument is identifying or that its the graphQL id

Can you clarify how this would be different than node?

It would make fetching some additional information very convenient,

Hmm. Can you clarify your goal here? There are two options for fetching data in a component, whichever is more convenient: use setVariables/forceFetch or manually construct a query and fetch it. The second option is easy to overlook, but you can always create a root query without a route and fetch it:

const query = Relay.createQuery(Relay.QL`query Foo { ... }`, variables); 
RelayStore.primeCache({query}, readyState => {
  if (readyState.error) {
    console.error(readyState.error);
  } else if (readyState.done) {
    // data is ready
  }
});

Note that this method will write the results of the query into the cache, and all affected components will re-render if necessary (i.e. you don't need to call setState or anything in the callback above).

@jardakotesovec

This comment has been minimized.

Show comment
Hide comment
@jardakotesovec

jardakotesovec May 5, 2016

Contributor

@josephsavona I apologize for vague description what I had in mind. Example should clear it up.

export default Relay.createContainer(ComponentA, {
    initialVariables: {
        formatIds: [],
    },
    fragments: {
        viewer: () => Relay.QL`
            fragment on Viewer {
                formats (ids: $formatIds) {
                    id
                    name
                    code
                }
            }
        `
    }
})

This is the on demand scenario when I want to get metadata for formats using setVariables. My use case is that I have some general informations for all formats (from initial 'static' query) but need to get some extra informations on demand on different places in the app and wanted to reduce over-fetching. So I know the ids for formats and want to get extra fields on-demand for few of them.

But afaik it is not possible to tell Relay that I am gonna use list of actual graphQL ids. Therefore it re-fetches format for any new combination of ids.

So lets say at first I would do:

  1. setVariables({formatIds: [id1, id2]}), relay fetches formats id1 and id2 with all required fields in this fragment instead of just missing fields.
  2. setVariables({formatIds: [id3, id2]}), relay fetches formats with id3 and id2 instead of just missing fields for id3

node field is the closest thing, but I can't use it for list of ids and use it in component fragment to get data from container.

Contributor

jardakotesovec commented May 5, 2016

@josephsavona I apologize for vague description what I had in mind. Example should clear it up.

export default Relay.createContainer(ComponentA, {
    initialVariables: {
        formatIds: [],
    },
    fragments: {
        viewer: () => Relay.QL`
            fragment on Viewer {
                formats (ids: $formatIds) {
                    id
                    name
                    code
                }
            }
        `
    }
})

This is the on demand scenario when I want to get metadata for formats using setVariables. My use case is that I have some general informations for all formats (from initial 'static' query) but need to get some extra informations on demand on different places in the app and wanted to reduce over-fetching. So I know the ids for formats and want to get extra fields on-demand for few of them.

But afaik it is not possible to tell Relay that I am gonna use list of actual graphQL ids. Therefore it re-fetches format for any new combination of ids.

So lets say at first I would do:

  1. setVariables({formatIds: [id1, id2]}), relay fetches formats id1 and id2 with all required fields in this fragment instead of just missing fields.
  2. setVariables({formatIds: [id3, id2]}), relay fetches formats with id3 and id2 instead of just missing fields for id3

node field is the closest thing, but I can't use it for list of ids and use it in component fragment to get data from container.

@Globegitter

This comment has been minimized.

Show comment
Hide comment
@Globegitter

Globegitter May 5, 2016

Contributor

It might even be nice to have a concept of nodes() field which would require either a type or ids param but also would allow for some additional optional filters so this could be used for fetching all nodes of the given type, fetch all nodes given the list of ids and have some additional customer filters (e.g. I want only nodes of a type that include a title)

Looking something like this:

const nodesQueryConfig = {
  queries: {
    nodes: () => {
      return Relay.QL`query { nodes(ids: $ids) }`
    }
  },
  name: 'nodesQueryConfig',
  params: {
    ids: [id1, id2]
  }
};

Or of course wrapped in the viewer.

Contributor

Globegitter commented May 5, 2016

It might even be nice to have a concept of nodes() field which would require either a type or ids param but also would allow for some additional optional filters so this could be used for fetching all nodes of the given type, fetch all nodes given the list of ids and have some additional customer filters (e.g. I want only nodes of a type that include a title)

Looking something like this:

const nodesQueryConfig = {
  queries: {
    nodes: () => {
      return Relay.QL`query { nodes(ids: $ids) }`
    }
  },
  name: 'nodesQueryConfig',
  params: {
    ids: [id1, id2]
  }
};

Or of course wrapped in the viewer.

@Globegitter

This comment has been minimized.

Show comment
Hide comment
@Globegitter

Globegitter May 5, 2016

Contributor

aahh @jardakotesovec have you seen the @relay(plural: true) option yet? That would allow you to ask for multiple ids and return you all these elements.

Contributor

Globegitter commented May 5, 2016

aahh @jardakotesovec have you seen the @relay(plural: true) option yet? That would allow you to ask for multiple ids and return you all these elements.

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona May 5, 2016

Contributor

node field is the closest thing, but I can't use it for list of ids and use it in component fragment to get data from container.

Relay supports plural identifying root fields which work exactly as you're describing. If you implement a nodes(ids: [ID!]): [Node] field in the schema, Relay understands that each of the id arguments corresponds to the specific Node with that id. If you fetch nodes(ids: [1,2]) and then nodes(ids: [2,3]), Relay will know that id 2 has already been fetched, and diff this to only fetch node(id: 3).

Contributor

josephsavona commented May 5, 2016

node field is the closest thing, but I can't use it for list of ids and use it in component fragment to get data from container.

Relay supports plural identifying root fields which work exactly as you're describing. If you implement a nodes(ids: [ID!]): [Node] field in the schema, Relay understands that each of the id arguments corresponds to the specific Node with that id. If you fetch nodes(ids: [1,2]) and then nodes(ids: [2,3]), Relay will know that id 2 has already been fetched, and diff this to only fetch node(id: 3).

@kamek-pf

This comment has been minimized.

Show comment
Hide comment
@kamek-pf

kamek-pf May 7, 2016

@josephsavona

Note that this method will write the results of the query into the cache, and all affected components will re-render if necessary (i.e. you don't need to call setState or anything in the callback above).

I actually tried that, I built a query to fetch some additional fields on a node and sent it to the node interface.
The query built by Relay looks correct (only the missing fields are asked), but the result is never merged into the cache.
The only way I found to get the updated data is Relay.Store.readQuery(query)[0];
Any idea what I'm doing wrong ?

kamek-pf commented May 7, 2016

@josephsavona

Note that this method will write the results of the query into the cache, and all affected components will re-render if necessary (i.e. you don't need to call setState or anything in the callback above).

I actually tried that, I built a query to fetch some additional fields on a node and sent it to the node interface.
The query built by Relay looks correct (only the missing fields are asked), but the result is never merged into the cache.
The only way I found to get the updated data is Relay.Store.readQuery(query)[0];
Any idea what I'm doing wrong ?

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona May 7, 2016

Contributor

@kamek-pf the public data-fetching APIs always merge data into the cache. If you're not seeing an update one of two things could be happening:

  • mismatched ID - maybe the component isn't listening on the id that is updated. To confirm this, check the earlier results and later results to make sure the IDs match
  • wrong API. it sounds like you're calling the right method, but if you paste your code we can help diagnose.

Either way, can you open a new issue for this? This is tangential to the OP.

Contributor

josephsavona commented May 7, 2016

@kamek-pf the public data-fetching APIs always merge data into the cache. If you're not seeing an update one of two things could be happening:

  • mismatched ID - maybe the component isn't listening on the id that is updated. To confirm this, check the earlier results and later results to make sure the IDs match
  • wrong API. it sounds like you're calling the right method, but if you paste your code we can help diagnose.

Either way, can you open a new issue for this? This is tangential to the OP.

@jardakotesovec

This comment has been minimized.

Show comment
Hide comment
@jardakotesovec

jardakotesovec May 9, 2016

Contributor

@josephsavona I am aware of the plural identifying root field, but I have two issues there

  • I need this field formats it on different places in the app inside components and I don't see how I could use them inside fragment, since its root field
  • And yes it allows to link keys with objects, but it does not know that its actual graphQL id, therefore if I get all formats (with some basic fields) at the beginning and want to get extra fields using root identyfing fields, it fetches all fields and creates additional mapping between id and object. This issue was mentioned here.
Contributor

jardakotesovec commented May 9, 2016

@josephsavona I am aware of the plural identifying root field, but I have two issues there

  • I need this field formats it on different places in the app inside components and I don't see how I could use them inside fragment, since its root field
  • And yes it allows to link keys with objects, but it does not know that its actual graphQL id, therefore if I get all formats (with some basic fields) at the beginning and want to get extra fields using root identyfing fields, it fetches all fields and creates additional mapping between id and object. This issue was mentioned here.
@LegNeato

This comment has been minimized.

Show comment
Hide comment
@LegNeato

LegNeato Jul 20, 2016

So, with the viewer workaround, how do we query the node root field if everything goes through viewer? Is this what the additional "self-pointer to my root type" workaround is used for? So you can do something like:

fragment on Viewer {
  root {
    node(id: $whatever) {
    ...
  }
}

But doing that gives me:

You defined a `node(id: ID!)` field on type `Query`, but Relay requires the `node` field to be defined on the root type.

LegNeato commented Jul 20, 2016

So, with the viewer workaround, how do we query the node root field if everything goes through viewer? Is this what the additional "self-pointer to my root type" workaround is used for? So you can do something like:

fragment on Viewer {
  root {
    node(id: $whatever) {
    ...
  }
}

But doing that gives me:

You defined a `node(id: ID!)` field on type `Query`, but Relay requires the `node` field to be defined on the root type.
@mjtamlyn

This comment has been minimized.

Show comment
Hide comment
@mjtamlyn

mjtamlyn Jul 20, 2016

@LegNeato The node field needs to be on Query, alongside your root node.

mjtamlyn commented Jul 20, 2016

@LegNeato The node field needs to be on Query, alongside your root node.

@LegNeato

This comment has been minimized.

Show comment
Hide comment
@LegNeato

LegNeato Jul 20, 2016

I have it there but still not getting how to query it from client code as the client fragments all refer to viewer.

const queryType = new GraphQLObjectType({
  name: 'Query',
  fields: () => ({
    node: nodeField,
    viewer: {
      type: viewerType,
    },
  }),
});

I'll poke at it some more and take it to stackoverflow if I still don't get it.

LegNeato commented Jul 20, 2016

I have it there but still not getting how to query it from client code as the client fragments all refer to viewer.

const queryType = new GraphQLObjectType({
  name: 'Query',
  fields: () => ({
    node: nodeField,
    viewer: {
      type: viewerType,
    },
  }),
});

I'll poke at it some more and take it to stackoverflow if I still don't get it.

@miracle2k

This comment has been minimized.

Show comment
Hide comment
@miracle2k

miracle2k Jul 21, 2016

@LegNeato I used to have my own copy of node called object() below of client. I believe the reason I named it "object" is the error you posted above. The check for the location of "node" that Relay does is, I think, to dumb to understand that this is an additional node field. Try to rename it.

However, have you considered (I am doing this now instead myself) just using two root queries:

export const relayRoute = (queries, paramsGetter) => BaseComponent => {
  // Generate a route
  class GeneratedRoute extends Relay.Route {
    static queries = queries;
    static paramDefinitions = {};
    static routeName = 'GeneratedRoute';
  }

  // Return a HOC that injects a route generated by routeGetter.
  return withProps(ownerProps => {
    // Let user's function generate the route
    const params = paramsGetter(ownerProps);
    return {relayRoute: new GeneratedRoute(params)};
  })(BaseComponent);
}
const route = relayRoute({
client: () => Relay.QL`
    query { client }
`,
menu: () => Relay.QL`
    query { menu: node(id: $id) }
`
})

miracle2k commented Jul 21, 2016

@LegNeato I used to have my own copy of node called object() below of client. I believe the reason I named it "object" is the error you posted above. The check for the location of "node" that Relay does is, I think, to dumb to understand that this is an additional node field. Try to rename it.

However, have you considered (I am doing this now instead myself) just using two root queries:

export const relayRoute = (queries, paramsGetter) => BaseComponent => {
  // Generate a route
  class GeneratedRoute extends Relay.Route {
    static queries = queries;
    static paramDefinitions = {};
    static routeName = 'GeneratedRoute';
  }

  // Return a HOC that injects a route generated by routeGetter.
  return withProps(ownerProps => {
    // Let user's function generate the route
    const params = paramsGetter(ownerProps);
    return {relayRoute: new GeneratedRoute(params)};
  })(BaseComponent);
}
const route = relayRoute({
client: () => Relay.QL`
    query { client }
`,
menu: () => Relay.QL`
    query { menu: node(id: $id) }
`
})
@LegNeato

This comment has been minimized.

Show comment
Hide comment
@LegNeato

LegNeato Jul 22, 2016

I ended up figuring it out, or at least something that works nicely. @miracle2k I think I can't just blindly use $id as some routes have multiple nested ids. Honestly didn't try it though.

In the schema definition:

// This is top-level / "root" node.
const queryType = new GraphQLObjectType({
  name: 'Query',
  fields: () => ({
    // The node query lets us query for any object with an id w/o having to manually define
    // queries under the viewer "global node". 
    node: nodeField,
   // This is the "global node" workaround mentioned above.
    viewer: {
      type: viewerType,
    },
  }),
});

export const Schema = new GraphQLSchema({
  query: queryType,
});

In the react-router definitions:

// This means when BarComponent is rendered, it will
// do a node query with the barID from the URL and query the node fields defined in
// the BarComponent component fragment named "node".
<Route path=":fooID/edit/:barID" component={BarComponent} queries={{
    viewer: () => Relay.QL`query { viewer }`,
    node: () => Relay.QL`query { node(id: $barID) }`,
 }} />

In the client component:

export default Relay.createContainer(BarComponent, {
  fragments: {
    // Define what we need to query from the viewer "global node".
    viewer: () => Relay.QL`
      fragment on Viewer {
        // Whatever fields...
      }
    `,
    // Define what we need to query from the node.
    node: () => Relay.QL`
      fragment on Node {
        id,
        ... on Bar {
            // Whatever fields...
        }
      },
    `,
  },
});

LegNeato commented Jul 22, 2016

I ended up figuring it out, or at least something that works nicely. @miracle2k I think I can't just blindly use $id as some routes have multiple nested ids. Honestly didn't try it though.

In the schema definition:

// This is top-level / "root" node.
const queryType = new GraphQLObjectType({
  name: 'Query',
  fields: () => ({
    // The node query lets us query for any object with an id w/o having to manually define
    // queries under the viewer "global node". 
    node: nodeField,
   // This is the "global node" workaround mentioned above.
    viewer: {
      type: viewerType,
    },
  }),
});

export const Schema = new GraphQLSchema({
  query: queryType,
});

In the react-router definitions:

// This means when BarComponent is rendered, it will
// do a node query with the barID from the URL and query the node fields defined in
// the BarComponent component fragment named "node".
<Route path=":fooID/edit/:barID" component={BarComponent} queries={{
    viewer: () => Relay.QL`query { viewer }`,
    node: () => Relay.QL`query { node(id: $barID) }`,
 }} />

In the client component:

export default Relay.createContainer(BarComponent, {
  fragments: {
    // Define what we need to query from the viewer "global node".
    viewer: () => Relay.QL`
      fragment on Viewer {
        // Whatever fields...
      }
    `,
    // Define what we need to query from the node.
    node: () => Relay.QL`
      fragment on Node {
        id,
        ... on Bar {
            // Whatever fields...
        }
      },
    `,
  },
});
@wincent

This comment has been minimized.

Show comment
Hide comment
@wincent

wincent Sep 3, 2016

Contributor

We're preparing Relay 2, which completely removes the restrictions and special casing around root fields. So I'm going to close this as we're unlikely to take any direct action on it, but people using Relay 1 will still be able to find this issue via search. Thanks to everybody who has participated in the thread!

Contributor

wincent commented Sep 3, 2016

We're preparing Relay 2, which completely removes the restrictions and special casing around root fields. So I'm going to close this as we're unlikely to take any direct action on it, but people using Relay 1 will still be able to find this issue via search. Thanks to everybody who has participated in the thread!

@josephsavona

This comment has been minimized.

Show comment
Hide comment
@josephsavona

josephsavona Nov 13, 2016

Contributor

@maletor That extra level of indirection isn't necessary. It's sufficient to do:

query {
  viewer {
    rootConnection
    multipleArgs(a: 1, b: 2)
  }
}
Contributor

josephsavona commented Nov 13, 2016

@maletor That extra level of indirection isn't necessary. It's sufficient to do:

query {
  viewer {
    rootConnection
    multipleArgs(a: 1, b: 2)
  }
}
@ermik

This comment has been minimized.

Show comment
Hide comment
@ermik

ermik Mar 4, 2018

@wincent I ran into an issue recently where createRefetchContainer refetching a root query prop wouldn't subscribe to the store. The refetch() calls would use the component configuration fragment and the query correctly, successfully refetching data, but the component would never get initial query props nor would they update on refetch. Tests in a dedicated <QueryRenderer /> as well as in a more complex component tree has shown this to be an issue. But once the query field was moved on a second level (e.g. viewer or anything else) — the component behaves as expected.

Problematic code:

{
    query: graphql`
      fragment RelaySelectComponent_query on Query
        @argumentDefinitions(s: { type: "String", defaultValue: "beer" }) {
        products(search: $s) {
          edges {
            node {
              id
              gtin
              description
            }
          }
        }
      }
    `
  },
  graphql`
    # Refetch query to be fetched upon calling refetch.
    # Notice that we re-use our fragment and the shape of this query matches our fragment spec. $count: Int,
    query RelaySelectComponentRefetchQuery($s: String = "wine") {
      ...RelaySelectComponent_query @arguments(s: $s)
    }
}

I am not as experienced as one (i.e. me) would hope, so I can't make it into a definitive bug report, but I'm asking, respectfully, you guys take a look. I wasted good 20 work hours trying to find a solution.

ermik commented Mar 4, 2018

@wincent I ran into an issue recently where createRefetchContainer refetching a root query prop wouldn't subscribe to the store. The refetch() calls would use the component configuration fragment and the query correctly, successfully refetching data, but the component would never get initial query props nor would they update on refetch. Tests in a dedicated <QueryRenderer /> as well as in a more complex component tree has shown this to be an issue. But once the query field was moved on a second level (e.g. viewer or anything else) — the component behaves as expected.

Problematic code:

{
    query: graphql`
      fragment RelaySelectComponent_query on Query
        @argumentDefinitions(s: { type: "String", defaultValue: "beer" }) {
        products(search: $s) {
          edges {
            node {
              id
              gtin
              description
            }
          }
        }
      }
    `
  },
  graphql`
    # Refetch query to be fetched upon calling refetch.
    # Notice that we re-use our fragment and the shape of this query matches our fragment spec. $count: Int,
    query RelaySelectComponentRefetchQuery($s: String = "wine") {
      ...RelaySelectComponent_query @arguments(s: $s)
    }
}

I am not as experienced as one (i.e. me) would hope, so I can't make it into a definitive bug report, but I'm asking, respectfully, you guys take a look. I wasted good 20 work hours trying to find a solution.

@ermik

This comment has been minimized.

Show comment
Hide comment
@ermik

ermik May 12, 2018

@yungsters maybe you can comment on this?

ermik commented May 12, 2018

@yungsters maybe you can comment on this?

@sibelius

This comment has been minimized.

Show comment
Hide comment
@sibelius

sibelius May 12, 2018

Collaborator

This is working fine on Relay Modern

Collaborator

sibelius commented May 12, 2018

This is working fine on Relay Modern

@ermik

This comment has been minimized.

Show comment
Hide comment
@ermik

ermik May 12, 2018

@sibelius the code snippet above was tested last in Relay 1.4.1 and was failing. I might be mistaken of course.

ermik commented May 12, 2018

@sibelius the code snippet above was tested last in Relay 1.4.1 and was failing. I might be mistaken of course.

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