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

[gatsby-transformer-javascript-frontmatter] Only create nodes from files that actually export frontmatter #4095

Closed
cwgw opened this issue Feb 16, 2018 · 3 comments · Fixed by #4100

Comments

@cwgw
Copy link
Contributor

cwgw commented Feb 16, 2018

Since we've switched from [gatsby-transformer-javascript-static-exports] to [gatsby-transformer-javascript-frontmatter], and limited the focus of the plugin to frontmatter in javascript files (as described by @jbolda in #3795), I'd like to suggest that the plugin only create nodes from javascript files that actually export frontmatter.

Right now, the plugin seems to create a javascriptFrontmatter node for each file with a media type of application/javascript before actually checking for the existence of exports.frontmatter. This can produce a lot of false positives, resulting in queries like…

{
  allJavascriptFrontmatter (
    filter: {frontmatter: {title: {ne: null}}}
  ) {
    edges {
      node {
        frontmatter {
          ...
        }
      }
    }
  }
}

Filtering in this way is not really a big deal, but it seems silly to have to explicitly exclude nodes that don't have frontmatter when you're querying for nodes that have frontmatter.

I don't think I'm capable of offering a PR. If there's some other way I can help, or if there's a use case for the current behavior that I'm missing, please let me know.

Thanks.

@KyleAMathews
Copy link
Contributor

Great idea!

You might very well be able to create a PR though :-) It'd just be adding an if check on the result of looking of looking for frontmatter.

@cwgw
Copy link
Contributor Author

cwgw commented Feb 16, 2018

Well, okay. I'll give it a shot. I appreciate the confidence.

@jbolda
Copy link
Contributor

jbolda commented Feb 16, 2018

Makes sense to me 👍

Thanks for the initiative!

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

Successfully merging a pull request may close this issue.

4 participants