Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

[API discussion] Allow dynamic query documents #168

Closed
rricard opened this issue Aug 24, 2016 · 7 comments
Closed

[API discussion] Allow dynamic query documents #168

rricard opened this issue Aug 24, 2016 · 7 comments

Comments

@rricard
Copy link
Contributor

rricard commented Aug 24, 2016

This is a partial followup to #163

Right now the new API (0.4+) forces you to define your query non-dynamically, which is the advised way of doing things. However, as stated in #163, allowing dynamism would be interesting for many reasons, even if it's hard to integrate, it would be an interesting escape hatch.

Note that at this time we have migrated to the new API but some of those concerns could always make our code better if they were solved:

  • Dynamic dispatching of query: not really a good idea but may be useful - the main way to reproduce that currently is with skip
  • Allow requiring external queries that rerequire fragments in the module using the query (aka a circular dependency): right now if you create a circular dependencies, bad things will happen. If you were however, to wrap the creation of the query dynamically in a function, this would disappear. (I know that something like rollup or webpack 2 should handle the issue by just requiring the right stuff at the right moment and not the whole module, basically making circular deps a thing of the past, but it's not quite there yet!) - a solution to that was to make placeholder queries with the same "signature" as the query or mutation but that will be replaced by a dynamically provided query later
@jbaxleyiii
Copy link
Contributor

@rricard could you give an example of Allow requiring external queries that rerequire fragments in the module using the query?

Dynamic dispatching of query: not really a good idea but may be useful - the main way to reproduce that currently is with skip

Do you want something different / in addition to skip? I was thinking we could add a client method on the data object to force query so you could skip: true then data.query() // or something on internal state change or what not

@rricard
Copy link
Contributor Author

rricard commented Aug 25, 2016

@rricard could you give an example of Allow requiring external queries that rerequire fragments in the module using the query?

Here is an example of a circular dep:

// components/Person.js

import {personQueryDoc, createPersonQuery} from '../queries/PersonQuery';

// ...

// I define fragments alongside my components for collocation
export const personFragment = createFragment(...);

// ...

// I use the shared query doc to reuse the same root query
const PersonWithData = graphql(personQueryDoc, { // I had to detach the static document
  options: (ownProps) => createPersonQuery(ownProps.personId), // from the query creation
})(Person);

export default PersonWithData;
// queries/PersonQuery.js

import {personFragment} from '../components/Person';

// ...

export const personQueryDoc = gql`...`;

export function createPersonQuery(personId) {
  return {
    query: personQueryDoc,
    variables: {personId},
    fragments: personQueryDoc,
  };
}

This may not work if webpack loads PersonQuery first (that will then load Person but PersonQuery will have personQueryDoc as undefined, resulting in a nice bug: can't find property kind of undefined)

@rricard
Copy link
Contributor Author

rricard commented Aug 25, 2016

Ultimately the solution we'll be doing here is to separate components from fragments but you have to acknowledge that can be blocking if you want to apply the structure I just demonstrated to you ...

@rricard
Copy link
Contributor Author

rricard commented Aug 25, 2016

To be clear, this is not an issue with apollo (here it is webpack + babel, webpack 2 or rollup should not raise the issue) but apollo doesn't help by requiring a static document...

@stubailo
Copy link
Contributor

Oh - I guess this is just referring to the fact that the whole static document has to be available at the time that the graphql function is called, making it hard to resolve certain dependency issues.

@nosovsh
Copy link

nosovsh commented Mar 1, 2017

Did I understand correctly that there is no way currency to set a query via props? And this will not be implemented by design? I'm talking about something like this:
<GraphqlComponent
query={gql...}
/>

@dahjelle
Copy link
Contributor

For future reference, #330 has some more discussion on this.

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

No branches or pull requests

5 participants