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

Dynamic queries passed into graphql() #330

Closed
chillu opened this issue Nov 17, 2016 · 33 comments
Closed

Dynamic queries passed into graphql() #330

chillu opened this issue Nov 17, 2016 · 33 comments

Comments

@chillu
Copy link

chillu commented Nov 17, 2016

We're converting data access for our PHP-based CMS UI into GraphQL. Naturally, the "active record" style data types managed by the CMS are user defineable (via PHP), so every installation of the CMS will have a different UI. We have a dynamic "form schema" which determines the form fields in a particular installation, and are hoping to use a GraphQL mutation for the form submission - with a 1:1 mapping between form fields and GraphQL fields. At the moment, we're not requiring PHP developers to install a JS build toolchain to achieve this level of customisation.

The graphql() HOC expects a gql document passed in when the script is first evaluated. We're looking for a way to allow dynamic definition of fields at this point, rather than bundling them into the JS build.

From what I can tell, the HOC doesn't actually use the query for anything before the component is constructed for the first time, the only query-related "initialisation" behaviour is const operation = parser(document). Would it be possible to pass in a Promise instead of Document into graphql(), which lazily returns a GraphQL query and defers any logic in the HOC until an instance of it is created? The same would be required for query-related options like fragments. I'm aware that this usage would bypass some of the static analysis benefits which GraphQL clients like Relay or Apollo have now, or build out in the future. In our case, we'd use this Promise to inspect a "fragment registry" which could be populated with server-generated JSON with varying fields.

If you don't think this is appropriate for the graphql() HOC, can you think of a "middle path" where we can still take advantage of components being aware of queries, without dropping straight down to a client.watchQuery() level?

There's been a similar discussion on Relay, around the need for babel-relay-plugin to access the schema at JS build time (rather than early runtime in Apollo). Note that the "JSON GraphQL Type" mentioned there won't cut it for us, since it negates a main benefit of GraphQL (strong typing).

Overall, I think a lot of CMS devs are getting excited about the expressive possibilities of GraphQL, but the lacking support for dynamic schemas will be a stumbling block for any customiseable UI that's not just building out single-deployment web applications.

Thanks for Apollo, it's been a great experience for us so far - we've actively evaluated it against Relay and found it to be a lot more friendly to use!

@wmertens
Copy link
Contributor

You could use that JSON graphql type and then https://github.com/hapijs/joi to verify the schema of the incoming data.

GraphQL is leaning towards static queries, so that they can be fetched efficiently…

@chillu
Copy link
Author

chillu commented Nov 21, 2016

Thanks for the response @wmertens! Adding a separate JSON schema on top of GraphQL sounds a bit like reinventing the wheel :) I'm having a play with queryTransformer in the client at the moment to add fields to queries after they've run through gql() tags. It's only half a solution to my problem, because sometimes I don't know which query to start with in the first place at JS evaluation time.

@stubailo
Copy link
Contributor

Hmm, I do like that the current API heavily recommends using static queries - after all, we think it's the right approach for most apps: https://dev-blog.apollodata.com/5-benefits-of-static-graphql-queries-b7fa90b0b69a#.axv02k49m

It sounds like you just want to have dynamic queries on app initialization time, rather than on each render of a component?

I think that could be somewhat complex because it makes the loading state of a component much harder to determine. First we have to wait for something to decide what the query should be, then send that to the server and wait for results.

Can you give me an example of the transformation you would want to do?

@chillu
Copy link
Author

chillu commented Nov 22, 2016

It sounds like you just want to have dynamic queries on app initialization time, rather than on each render of a component?

That's the largest use case for us. For example, we might have a hello <username> component in the CMS toolbar. This component would have a co-located query to retrieve the username field. A third-party dev might want to add another component which shows a profile picture next to it, as a CMS module which other devs can use in their own installations. Which means the additional profilePictureUrl field should be retrieved through an existing query. While I can co-locate a new fragment with this new component, I would still need the parent component to add the fragment to the existing query.

We also have a "form builder" component which retrieves a "form schema" on first render of the component, which determines the rendered fields. Forms can be loaded by the user navigating around the UI. In an ideal world we could use this schema to construct a form submission query at render rather than initialisation time. We're working around this through a generic submitForm query which has an array of name/value keys - so this use case is less important (since it's going to be harder to resolve through Apollo).

@chillu
Copy link
Author

chillu commented Nov 23, 2016

I've discovered that queryTransformer is deprecated (it's not marked as such in your docs: http://dev.apollodata.com/core/apollo-client-api.html#ApolloClient.constructor). Which gets me back to square one regarding my approach unfortunately. What's the rationale for removing this feature (see apollographql/apollo-client#779)? You only had a single use case for it (add __typename) which is now covered directly through the API, so I understand the drive to simplify the API surface - but it doesn't help me in my current predicament :D Are query transformations (modifying the gql AST) supported through other means?

@chillu
Copy link
Author

chillu commented Nov 24, 2016

My proposal for a "fragment registry" in our CMS UI might clarify our use case a bit: silverstripe/silverstripe-graphql#13

@chillu
Copy link
Author

chillu commented Dec 7, 2016

@stubailo Would you be open to adding the queryTransformer API back into Apollo? It looks like you still have a use case for the concept (add __typename dynamically), it's just more locked down now. That's a blocker for us in adopting Apollo in our CMS, since we can't sacrifice the existing extensibility of its UI and data fetching.

@stubailo
Copy link
Contributor

stubailo commented Dec 7, 2016

@chillu what's your use case?

@stubailo
Copy link
Contributor

stubailo commented Dec 7, 2016

Oh - I see there are a lot of comments above. I'll defer to @helfer.

@stubailo
Copy link
Contributor

stubailo commented Dec 7, 2016

Can you build a custom version of graphql-tag that modifies queries after parsing them? Basically, I don't think something needs to be built into apollo-client to modify queries for you, our hope is to eventually remove all query processing from AC (for example now fragment management is part of graphql-tag)

@chillu
Copy link
Author

chillu commented Dec 11, 2016

Thanks for your feedback @stubailo! I've solved part of our problem now by creating a "lazy HOC" around graphql(), without requiring any modifications to your library. Would you be able to sanity check my approach here? I haven't seen HOC initialised in this way before, but it seems to work fine.

register = new GraphQLRegister();

export function graphqlWithRegister(document, operationOptions) {
  return function wrappedGraphQLWithRegister(WrappedComponent) {
    let ComponentWithGraphQL; // Closed-over value is scoped to component
    class WrappedGraphQLWithRegister extends Component {
      constructor() {
        super();

        // Lazy initialising of HOC to give customisations an opportunity to be registered
        if (!ComponentWithGraphQL) {
          ComponentWithGraphQL = graphql(
            // Adds customisations which have been registered by third party code
            register.transformDocument(document, operationOptions),
            register.transformOptions(document, operationOptions)
          )(WrappedComponent);
        }
      }
      render() {
        return <ComponentWithGraphQL {...this.props} />;
      }
    }
    WrappedGraphQLWithRegister.displayName = `WrappedGraphQLWithRegister(${getDisplayName(WrappedComponent)})`;

    return WrappedGraphQLWithRegister;
  };
}

Check GraphQLRegister.js for a really rough but working version of this (and more context on our issue tracker).

The other part (dynamic queries at runtime) would need to be solved by a variation of the react-apollo library where we can watch arbitrary queries based on runtime logic, and build in similar smarts to keep the component updated. That might lead to further enquiries here, or an alternative approach to react-apollo, haven't thought it through yet.

Anyway, I'm closing this since you made the intentions of the library clear - thanks again for responding.

@chillu chillu closed this as completed Dec 11, 2016
@bbbbighead
Copy link

hi @chillu , I found that I have same demands like yours. Is there any official way in react-apollo supporting dynamic graphql query at run time? Please help and give me some advise. Happy Christmas!! Thanks. :)

@chillu
Copy link
Author

chillu commented Jan 8, 2017

@bbbbighead I can't comment on the roadmap for react-apollo, but from what the maintainers have said before I'm not getting my hopes up for dynamic query support there. We might end up creating our own variation of react-apollo, for now we've parked the issue since we've derisked the issue for our use case.

@dahjelle
Copy link
Contributor

Does anyone have a suggested technique for passing dynamic queries into react-apollo? This was supported in 0.3.19 (a long time ago :-) ), but I don't see an obvious way of accomplishing it with current versions of react-apollo. I suppose one could just use the watchQuery of apollo-client directly…

@dahjelle
Copy link
Contributor

From a discussion on Slack, if you need to do a dynamic query, you can use another higher-order component, like this:

function buildQueries(ComponentToWrap) {
  return function(props) {
    const query = /*build your query AST here*/;
    const Wrapped = graphql(query)(ComponentToWrap);
    return <Wrapped {...props}/>;
  };
}
export default addDynamicQuery(MyComponent);

@nosovsh
Copy link

nosovsh commented Apr 24, 2017

@dahjelle as far as I can see this method could bring really bad problems. New instanc of Wrapped component will be created from scratch on each rerender of addDynamicQuery(MyComponent). That means that if MyComponent or any deeper placed components are using state it will be reset to default value on each rerender. That's why you can not use graphql, connect or any other similar hoc inside render.

Or am I missing something?

@dahjelle
Copy link
Contributor

dahjelle commented Apr 24, 2017

Thanks for the feedback, @nosovsh! I appreciate hearing from folks with more expertise than myself. :-D

I don't disagree…I frankly don't know enough about that part of React.

state it will be reset to default value on each rerender

Makes sense. I suppose the usual way of dealing with that is putting all of one's state in Redux or such like?

New instanc of Wrapped component will be created from scratch on each rerender of addDynamicQuery(MyComponent).

This, too, makes sense. Do you have any data as to the performance impact this would have for stateless components?


Are you aware of an approach for dynamic queries that you'd recommend? Perhaps @chillu's approach above?

@tnrich
Copy link

tnrich commented Apr 26, 2017

Hey everyone, I have a use case I'd like to support with apollo/graphql:

Basically the idea is to allow users to define which columns (from a set list) they would like to view on a data table. Querying for all of the columns could mean a lot of unnecessary data would be grabbed, thus, I'd like to only send a graphql query with the user-requested columns.

I haven't been able to figure out how to do this yet, but I might be missing something. Any help would be greatly appreciated!

Also, this seems like a pretty common use case. I understand that apollo/relay are moving toward static queries (which might make sense for 90% of use cases), but it seems unwise to completely drop all dynamic query functionality (sometimes you just need to do it).

@nosovsh
Copy link

nosovsh commented Apr 27, 2017

@dahjelle
Storing state in redux will solve the issue with loosing state but it is more like a hack because component sub tree can be big and you can not be sure that nothing deeper is using local state.

Regarding performance I don't know what React will do exactly but my guess it will run the full life cycle for new component on each rerender (constructor, onComponentDidMount etc). And you will not have much profit from functional components. But I could be wrong here please check it yourself.

I'm writing this because we were doing the same and finally faced bug with loosing local state that was reeeally hard to catch :) After that we just switched from fully dynamic queries (sent by props) to hoc generators. Our queries are static but you can use the same component with different queries.

@tnrich
Copy link

tnrich commented Apr 27, 2017

@nosovsh , could you point to a repo or share code for your dynamic query solution using HOC generators? I'd be curious to see how you're doing it. Thanks!

@dahjelle
Copy link
Contributor

dahjelle commented May 2, 2017

For whatever it's worth, I'm trying a technique like @chillu's above. In my case, the queryBuilder is a function that takes props and returns an object of the shape {query, config} which are then passed directly to the graphql HOC.

import React, {Component} from 'react';
import {graphql} from 'react-apollo';

export default function wrapDynamicGraphQL(queryBuilder, ComponentToWrap) {
  class Wrapped extends Component {
    constructor(props) {
      super(props);
      const {query, config = {}} = queryBuilder(props);
      this.wrapped = graphql(query, config)(ComponentToWrap);
    }

    render() {
      const Wrapped = this.wrapped;
      return <Wrapped {...this.props} />;
    }
  }

  return Wrapped;
}

The main limitation I can see is that, while the query is dynamic on instantiation, changing the props won't change the query. That's fine in my case, but wouldn't necessarily be fine in every case. It does fix the issue @nosovsh raised about loosing state as far as I can tell.

Other pros/cons that anyone sees?

I think that, if you need your queries to change in response to props without re-instantiating all child components, one would have to wrap using withApollo and using watchQuery directly. No idea how that might work in practice.

@nosovsh
Copy link

nosovsh commented May 3, 2017

I just found a note in official docs about loosing state if using hoc inside render: https://facebook.github.io/react/docs/higher-order-components.html#dont-use-hocs-inside-the-render-method
@dahjelle you are right, your solution is good until you need to change query on the fly. Of course you can check for changes in componentWillReceiveProps and reintialize this.wrapped if query is changed but then you again loosing local state of nested components.
@tnrich here is example of hoc that I use https://gist.github.com/nosovsh/6469f59e60406aacb8be07eae24c0109

@stubailo
Copy link
Contributor

stubailo commented May 3, 2017

@tnrich please open a new issue for this! By the way Apollo supports any kind of queries, not just static ones. So it should be easy to do what you're looking for.

@tnrich
Copy link

tnrich commented May 4, 2017

@dahjelle , what do you use for the queryBuilder in your above example?

@dahjelle
Copy link
Contributor

dahjelle commented May 5, 2017

@tnrich Something like the following (simplified a lot from our real code, but hopefully better gets the point across):

import {parse} from 'graphql';

export function queryBuilder(props) {
  const query = parse(`
    {
      person {
        ${props.column_name}
      }
    }
  `);
  return {
    query,
    config: {}
  };
}

You can pass any of the regular react-apollo stuff in the config, and there are a lot of different ways to build the query. Building the AST directly instead of parsing from text may be more reliable.

@EyMaddis
Copy link

In my setup, my component will no longer rerender when the loading state changes, only when new results are rendered.
Did somebody else notice this issue?

@mattyfresh
Copy link

mattyfresh commented Sep 21, 2017

I ran into this same issue today. My solve was to set the options param in the second argument to graphql as a function, as this function receives the props passed into said component. IE in my example I have a Post component which receives a name prop which I want to pass into a GraphQl query. You simply set your query string to receive variables as per the Apollo Docs (https://www.learnapollo.com/tutorial-react-native/react-native-03), and pass your variables into the object returned by your options function.

Example:

const recentPosts = gql`
query ShowsQuery($superHero: String!) {
    shows(superHero: $superHero) {
        show {
            name
            id
            premiered
            image {
                medium
            }
        }
    }
}
`

// The `graphql` wrapper executes a GraphQL query and makes the results
// available on the `data` prop of the wrapped component (PostList)
export default graphql(recentPosts, {
    options: (ownProps) => {
        return { variables: { superHero: ownProps.name } }
    },
    props: ({ data, ownProps }) => {
        return { data, ...ownProps }
    }
})(Post)

Now all instances of $superHero within your query string will be set to whatever ownProps.name is. AFAIK there is no real need for a HOC or anything fancy, this is straight from the docs.

@EyMaddis
Copy link

@mattyfresh This is not the problem.
Your query is actually static, a dynamic query means that the whole query string changes, as from above:

`
    {
      person {
        ${props.column_name} // this will lead to query
      }
    }
  `

@mattyfresh
Copy link

Ah I see, didn't understand what you meant by 'dynamic'. Would be great if you could just pass a function that returns a gql doc as the first argument of the graphql HOC 😬

@shtse8
Copy link

shtse8 commented Nov 1, 2017

@EyMaddis So how can I do the dynamic query? I have tried but an error is thrown.

@lucasconstantino
Copy link

I made a short try on this the other day. This is completely possible, and could easily replace some directives, like to only request some fields if a condition is match.

import React from 'react'
import { compose, withState } from 'recompose'
import { graphql } from 'react-apollo'
import gql from 'graphql-tag'

import withData from 'app/lib/withData'

const gqlDynamic = (strings, ...expressions) => ({ strings, expressions })

const dynQuery = gqlDynamic`
  query {
    currentUserContext {
      ${({ withId }) => withId ? 'uid' : ''}
      entityLabel
      name
    }
  }
`

const graphqlDynamic = ({ strings, expressions }, ...args) => ComposedCmoponent => class GraphQLProps extends React.Component {
  resolveExpression = expression => typeof expression === 'function'
    ? this.resolveExpression(expression(this.props))
    : expression

  render () {
    const query = gql(strings, expressions.map(this.resolveExpression))

    const ConnectedComposedCmoponent = compose(
      graphql(query)
    )(ComposedCmoponent)

    return (
      <ConnectedComposedCmoponent { ...this.props } />
    )
  }
}

// eslint-disable-next-line react/prop-types
const Page = ({ setWithId, withId, ...props }) => (
  <div>
    <button onClick={ () => setWithId(!withId) }>Toggle user id!</button>
    <pre>{ JSON.stringify(props, null, 2) }</pre>
  </div>
)

export default compose(
  withData,
  withState('withId', 'setWithId', false),
  graphqlDynamic(dynQuery),
)(Page)

Note that withData here is handling Apollo provider logic. Note also that the component is being composed inside rendering function, which is a HUGE problem, but works for demonstration purposes.

@CrocoDillon
Copy link

I wanted to share my take on this to help others and to get feedback.

It's based on the proposed Query component for version 2.1 from the roadmap which looks like this:

<Query
  skip={boolean}
  options={QueryOptions}
  loading={(result?) => Component || null}
  error={(result?) => Component || null}
  render={(result) => Component}
/>

So this is what I came up with and hopefully when version 2.1 comes refactoring to react-apollo's Query component will be really easy.

import React from "react";
import { bool, func, number, object, oneOf, oneOfType, string } from "prop-types";
import gql from "graphql-tag";
import { graphql } from "react-apollo";

// Cache of QueryContainer components based on parsed query or query string
const cache = new Map();

const QueryComponent = props => {
  const { data, loading, error, render } = props;

  if (data.networkStatus === 1) {
    // Query has never been run before
    return loading(data);
  }

  if (data.networkStatus === 8) {
    // Error, with no request in flight
    return error(data);
  }

  if (data.error) {
    // Error, but there is a request in flight...
    // ...let's hope for the best!
    return loading(data);
  }

  // data.loading might be true, but we can still show cached data
  return render(data);
};

QueryComponent.displayName = "QueryComponent";

const Query = props => {
  const { query } = props;

  if (!cache.has(query)) {
    const parsedQuery = typeof query === "object" ? query : gql(query);

    cache.set(query, graphql(parsedQuery, {
      skip: ({ skip }) => skip,
      options: ({ options }) => options,
    })(QueryComponent));
  }

  const QueryContainer = cache.get(query);

  return <QueryContainer {...props} />;
};

Query.displayName = "Query";

Query.propTypes = {
  query: oneOfType([string, object]).isRequired,
  skip: bool,
  options: object,
  loading: func,
  error: func,
  render: func.isRequired,
};

Query.defaultProps = {
  loading: () => null,
  error: () => null,
};

export default Query;

@seangransee
Copy link

I wanted to do the same thing @EyMaddis mentioned in #330 (comment).

I ended up publishing a simple wrapper around react-apollo that allows for a function that generates a query from your props.
https://www.npmjs.com/package/react-apollo-dynamic-query

If you don't want to include another dependency, you can just copy the function out of the source code.

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