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

Add variables attr into StaticQuery #9047

Open
engilyin opened this Issue Oct 12, 2018 · 16 comments

Comments

Projects
None yet
6 participants
@engilyin
Copy link

engilyin commented Oct 12, 2018

Summary

According to docs (at
https://github.com/gatsbyjs/gatsby/blob/master/docs/docs/static-query.md#how-staticquery-differs-from-page-query):

StaticQuery does not accept variables (hence the name "static"), but can be used in any component, including pages

However it could be very useful to add yet additional attribute to pass variables into the query.

Or may be add DynamicQuery to use queries inside the components with the variables.

Basic example

The variables attribute may contain single default variable, array of variables or map with varName:varValue pairs.

export default props => (
  <StaticQuery
    variables={this.props.pageContext.slug}
    query={
      `query ArticleBySlugEn( $slug: String! ) {
        markdownRemark( fields: { slug: { eq: $slug } } ) {
          html
          frontmatter {
            title
            date
          }
          fields {
            slug
          }
        }
      }
    `}
    render={data => <Header data={data} {...props} />}
  />
);

Motivation

I have multi language site with markdown pages. I need one common template for all pages. It is a component with a query. This query has a variable (slug). And I have page templates for each used language which I want to maintain as little as possible. But due to limitations of Gatsby 2.0 to query with variables from the components I just found I need to move query up and clone it into each of my top level language specific page template.
It is working somehow but I see warning on the console like this:

The GraphQL query in the non-page component "/path/to/ArticleTemplate.js" will not be run.
Exported queries are only executed for Page components. Instead of an exported
query, either co-locate a GraphQL fragment and compose that fragment into the
query (or other fragment) of the top-level page that renders this component, or
use a in this component. For more info on fragments and
composition, see http://graphql.org/learn/queries/#fragments and for more
information on , see https://gatsbyjs.org/docs/static-query

Probably I can reduce boilerplate using the fragments. But anyway I have to put some query code into each language specific top(page) component.

I prefer to forward the this.props.pageContext into my common template component and use it in StaticQuery.

@kakadiadarpan

This comment has been minimized.

Copy link
Contributor

kakadiadarpan commented Oct 12, 2018

@engilyin at the moment, StaticQuery isn't intended to accept GraphQL arguments and adding that functionality isn't on our roadmap. We'd be happy to see a pull request if someone wants to tackle this.

/cc @jlengstorf

@jlengstorf

This comment has been minimized.

Copy link
Member

jlengstorf commented Oct 13, 2018

@engilyin I agree that this would be really useful! However, the challenge is in the way StaticQuery is evaluated.

During the build, Gatsby looks for instances of StaticQuery in the AST and executes those queries in place. By accepting variables, we would need to trace the variables back to where they're set so we could pull those values out for evaluation, which is a non-trivial amount of effort.

To reiterate @kakadiadarpan, we'd absolutely love to get a pull request to implement this feature if someone has time to tackle it.

@engilyin

This comment has been minimized.

Copy link
Author

engilyin commented Oct 13, 2018

@kakadiadarpan and @jlengstorf thank you for a quick response! I just get started to use Gatsby and its internals are not so clear for me to help with pull request at this level.
However what about another idea to create DynamicQuery and execute it when variables are known?
Also there is an idea to define some global(page-wide) variables which would be know before you will process StaticQueries. E.g., when we use gatsby-node.js and createPage we may define context and its variables are used for global queries. Are you calling the execution of StaticQueries before or after I call createPage? If after(and I believe that) the context variables should be already easily available? Don't it?

I'm sorry if my ideas looks strange I previously used mostly Java JSF so could simply have misconceptions about how works ReactJS/Gatsby component model.

@jlengstorf

This comment has been minimized.

Copy link
Member

jlengstorf commented Oct 13, 2018

We could add a dynamic query to pages, but components are trickier. The benefit of StaticQuery is that it can be used anywhere, not just in pages.

Pages make it more predictable for us where the variables are set; with dynamic queries in components it's harder to determine where that data came from.

However, it's definitely possible; just not under active development right now. PRs welcome!

@ekdev2

This comment has been minimized.

Copy link

ekdev2 commented Nov 3, 2018

on the create pages API how can you do below if youre only allowed to use StaticQuery and it cant read the context.

).then(result => {
      result.data.allMarkdownRemark.edges.forEach(({ node }) => {
        createPage({
          path: node.fields.slug,
          component: path.resolve(`./src/templates/blog-post.js`),
          context: {
            // Data passed to context is available
            // in page queries as GraphQL variables.
            slug: node.fields.slug,
          },
        })
      })
      resolve()
    })
  })
@jlengstorf

This comment has been minimized.

Copy link
Member

jlengstorf commented Nov 3, 2018

@ekdev2 You can use standard queries with context on pages. If you need to access context, use a page query.

StaticQuery isn't a requirement, and you're allowed to use either page queries or StaticQuery in any combination.

Does that make sense?

@ekdev2

This comment has been minimized.

Copy link

ekdev2 commented Nov 3, 2018

@jlengstorf thanks for the response. On this section https://www.gatsbyjs.org/docs/static-query/#how-staticquery-differs-from-page-query it says "page queries can accept variables (via pageContext) but can only be added to page components". So in the section where I create pages programmatically (gatsby-node), can I still use components that reside in /src/templates ie. /src/templates/blog-post.js and that be considered a page component?

@jlengstorf

This comment has been minimized.

Copy link
Member

jlengstorf commented Nov 6, 2018

@ekdev2 Yes! If you supply a component to createPage, it treats that component as a page component. Under the hood, Gatsby is effectively calling createPage for each component in the src/pages/ directory and doing exactly that.

Hope that helps!

@rwieruch

This comment has been minimized.

Copy link
Member

rwieruch commented Dec 28, 2018

What's the best practice in the end when I need to pass a variable to the GraphQL query (that's only applicable to a page query and not StaticQuery as I understand it) but the command line warning says I should use StaticQuery or co-located GraphQL fragments. I guess then there is no way around fragments to avoid the warning for dynamically created pages?

@jlengstorf

This comment has been minimized.

Copy link
Member

jlengstorf commented Dec 28, 2018

@rwieruch the warning should be suppressed on dynamic pages. If the createPage call uses the component with the page query, it should work without warnings.

If that's what you're doing, could you share the code that's producing the warnings so we can take a look?

Thanks!

@rwieruch

This comment has been minimized.

Copy link
Member

rwieruch commented Dec 30, 2018

EDIT: I think in the case of my issue it may be MDX related.

@jlengstorf thanks for your reply. Yes, that's why I was wondering about the warning. I get it for one of my templates:

The GraphQL query in the non-page component "/Users/rwieruch/Developer/Blogs/rwieruch/src/templates/post.js" will not be run.
Exported queries are only executed for Page components. Instead of an exported
query, either co-locate a GraphQL fragment and compose that fragment into the
query (or other fragment) of the top-level page that renders this component, or
use a <StaticQuery> in this component.

As the warning says, the file sits in the src/templates/post.js folder. I create the post pages in the gatsby-node.js file:

createPage({
    path: node.fields.slug,
    component: componentWithMDXScope(
      path.resolve(`./src/templates/post.js`),
      node.code.scope,
      __dirname,
    ),
});

I guess, the only difference from this template compared to the other templates is the MDX, because I don't get the warning for my other dynamic pages.

Further Observations:

Even though the warning shows up, it works with the MDX when defining the page query as the following for the dynamic page:

export const pageQuery = graphql`
  query($id: String!) {
    site {
      siteMetadata {
        siteUrl
      }
    }
    post: mdx(fields: { id: { eq: $id } }) {
      frontmatter {
        title
      }
    }
  }
`;

However, if I change the variable name (e.g. from pageQuery to query) it will not work, because I never receive the data prop in props in my component. Also when I give the query a query name (e.g. query Post($id: String!) {) I will get an error in the command line even though the query name wasn't used somewhere else:

error GraphQL Error There was an error while compiling your site's GraphQL queries.
  Invariant Violation: GraphQLCompilerContext: Duplicate document named `Post`. GraphQL fragments and roots must have unique names.
    t: Duplicate document named `Post`. GraphQL fragments and roots must have unique names.

So maybe the whole MDX in Gatsby is doing some more work under the hood here. But that's just my assumption.

@jlengstorf

This comment has been minimized.

Copy link
Member

jlengstorf commented Dec 31, 2018

@rwieruch Yep, that's an MDX bug: ChristopherBiscardi/gatsby-mdx#214

It's been fixed (ChristopherBiscardi/gatsby-mdx#221), but it's not released under the gatsby-mdx@latest tag just yet. In the meantime, you can safely ignore the warning.

The pageQuery issue is also a known MDX issue (ChristopherBiscardi/gatsby-mdx#203) that's fixed by the same PR above.

MDX is still in early days — please follow that repo and open issues if you hit more MDX trouble!

Thanks!

@rwieruch

This comment has been minimized.

Copy link
Member

rwieruch commented Jan 1, 2019

Thank you @jlengstorf for the thorough answer! Helps me a lot :) Keep up the good work!

@gatsbot gatsbot bot added the stale? label Feb 10, 2019

@gatsbot

This comment has been minimized.

Copy link

gatsbot bot commented Feb 10, 2019

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

@jlengstorf jlengstorf added not stale and removed stale? labels Feb 10, 2019

@djfarly

This comment has been minimized.

Copy link

djfarly commented Feb 19, 2019

Just a thought: Wouldn't it be helpful if <StaticQuery /> could use the same variables from pageContext that the page query can? 🤔
You'd need to evaluate each StaticQuery for each page it exists in once.

createPage({
  ...,
  context: {
    someVar: 'Foo',
  },
})
// warning: this is an imaginary api and not working code

function SomeComponent() {
  const data = useStaticQuery(graphql`query ($someVar: String!) { … }`)
  ...
}

Does this make sense?
Would it be helpful to have this? I feel like it would cover at least some use cases and should be quite a bit simpler to implement than fully dynamic variables.

@djfarly

This comment has been minimized.

Copy link

djfarly commented Feb 19, 2019

Having said the above: One thing about Gatsby that super confused me in the beginning (and still is) are the page queries. They somehow magically provide data to a component just by being declared.

Consider this current api:

export default function MyPage({ data }) {
  return <div>{data.page.title}</div>;
}

export const query = graphql`query MyPage($id: Int!) { page(id: $id) { title } }`;

When learning this I asked myself:

  • does the query const has to be named query?
  • does it need to be exported?
  • does the query has to have the same name as the component?
  • how does gatsby get the data prop into my component (since there is no explicit connection besides them being in the same file)?

Now imagine the page query api would not exist and you were to use a static query instead:

// warning: this is an imaginary api and not working code

export default function MyPage() {
  const data = useStaticQuery(graphql`query MyPage($id: Int!) { page(id: $id) { title } }`);
  return <div>{data.page.title}</div>;
}

This feels a lot more explicit and potentially less confusing to me.

I think it only makes complete sense with a hook though.

I'm sorry if this is a little bit out of scope for this issue - do you think this could qualify as a rfc maybe?

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