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

Improve query parsing to recognize GraphQL queries outside export statements #3289

Closed
jbaxleyiii opened this issue Dec 20, 2017 · 8 comments
Closed

Comments

@jbaxleyiii
Copy link

Currently, Gatsby uses a combination of AST introspection and tagged template literal extraction to find and extract all of the graphql queries that pages can use. This works well if you have something that looks like this:

export const pageQuery = graphql`
  query Think {
     siteMetadata {
       title
    }
  }
`

The problem arises if you deviate from this at all. There are no warnings or errors, just no data passed to the component. This came up when working with ReasonML and Gatsby. Bucklescript can generate es6 code (though this opens up other issues like needing to compile node_modules) but what it creates is this (totally valid)

var pageQuery = graphql`
  query Think {
     siteMetadata {
       title
    }
  }
`

export { pageQuery }

Even though this is valid ES6, Gatsby can't find the query because the ExportNamedDeclaration isn't where the tagged template literal actually is found.

To further the issue, Gatsby no longer works with commonjs exports (41c5ce6) so a much simpler solution of exports.pageQuery = graphql will not work either.

@KyleAMathews I don't know babel transforms well enough to immediately suggest a solution, but would be happy to help with some guidance here.

For anyone who runs into this, the current work around with something like Reason, is to generate pages outside of the pages folder, them import the components using a normal .js file where you place the query. This isn't great at all, but at least it works.

@KyleAMathews
Copy link
Contributor

@jquense do you remember any reasons why we can't support other ways of exporting the query?

AFAIK, it's just that someone needs to write the Babel code to check other ways of exporting a graphql template literal.

@jbaxleyiii interested in working on a PR? The AST stuff is pretty decent to work with https://astexplorer.net/#/gist/907f90b010c04571772c1ae309d1c5a6/84df84d3057777606e5679add1aabcba98f2c0c9

@jquense
Copy link
Contributor

jquense commented Dec 20, 2017

There isn't a strong technical reason why it couldn't be different. The existing strategy was just straightforward and statically bullet proof, as well as similar to what the gatsby convention was already. We can definitely adjust it to cover more cases, however, we want to make sure that is fast and not prone to false positives/negatives. As it is query parsing is a big additional parsing cost so we want to make sure we are spending as little time possible doing it.

@calcsam calcsam changed the title Improve query parsing Improve query parsing to recognize GraphQL queries outside export statements Dec 26, 2017
@aaronsky
Copy link

aaronsky commented Jan 3, 2018

I would personally love more flexibility on this, especially as far as moving the graphql queries to separate .graphql files. I'm using Typescript with my site, and since I don't get any typing information from queries, this is what my types end up looking like:

interface IndexPageProps {
    data: {
        site: {
            siteMetadata: {
                title: string;
            };
        };
        allMarkdownRemark: {
            edges: MarkdownEdge[];
        };
    };
}

export default ({ data }: IndexPageProps) => (
    <div>
        {data.allMarkdownRemark.edges.map(renderBlogPost)}
    </div>
);

export const pageQuery = graphql`
  query IndexQuery {
    site {
      siteMetadata {
        title
      }
    }
    allMarkdownRemark(sort: { fields: [frontmatter___date], order: DESC }) {
      edges {
        node {
          excerpt
          fields {
            slug
          }
          frontmatter {
            date(formatString: "DD MMMM, YYYY")
            title
          }
        }
      }
    }
  }
`;

It's a common TS pattern to keep prop interface types either in the component module they're used in, or in a module at the same folder level. It would be great to have similar options for GraphQL queries, rather than being required to match these semantics exactly.

@wyze
Copy link

wyze commented May 3, 2018

I was able to get this to work fine with Bucklescript once I removed the ast check for the export statement. My diff is this:

        traverse(ast, {
-          ExportNamedDeclaration(path, state) {
-            path.traverse({
              TaggedTemplateExpression(innerPath) {
                const gqlAst = getGraphQLTag(innerPath)
                if (gqlAst) {
                  gqlAst.definitions.forEach(def => {
                    if (!def.name || !def.name.value) {
                      report.panic(getMissingNameErrorMessage(file))
                    }
                  })

                  queries.push(...gqlAst.definitions)
                }
              },
-            })
-          },
        })

@KyleAMathews
Copy link
Contributor

Due to the high volume of issues, we're closing out older ones without recent activity. Please open a new issue if you need help!

@ctbucha
Copy link

ctbucha commented Nov 1, 2018

I just ran into this issue today using ReasonML.

@ctbucha
Copy link

ctbucha commented Nov 1, 2018

From @jbaxleyiii 's advice, my current workaround is to use a regular ReactJS component and then just immediately pass the data down to the ReasonReact component.

import React from 'react'
import { graphql } from 'gatsby'
import MarkdownContent from './MarkdownContent.bs'

export default ({ data }) => { 
  console.log(data)
  const title = data.markdownRemark.frontmatter.title
  const content = data.markdownRemark.html
  return <MarkdownContent title={title} content={content} />
}

export const query = graphql`
  query BlogPostBySlug($slug: String!) {
    site {
      siteMetadata {
        title
      }
    }
    markdownRemark(fields: { slug: { eq: $slug } }) {
      id
      excerpt
      html
      frontmatter {
        title
        date(formatString: "MMMM DD, YYYY")
        author
      }
    }
  }
`

@loudmouth
Copy link

I'm a bit intimidated by making a PR on the Gatsby AST parsing, but I just wanted to Plus 1 that it would be awesome to enable Gatsby GraphQL queries in Reason files!

This was referenced Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants