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

Templated string not interperted in builds #5069

Closed
sqrtroot opened this issue Apr 21, 2018 · 21 comments
Closed

Templated string not interperted in builds #5069

sqrtroot opened this issue Apr 21, 2018 · 21 comments

Comments

@sqrtroot
Copy link

@sqrtroot sqrtroot commented Apr 21, 2018

Description

When using a templated string to filter for published articles on production builds like this

export const query = graphql`
query data{
  allMarkdownRemark(
    filter: {fileAbsolutePath: {regex: "/.*/blogs/.*/"}
            ${ process.env.NODE_ENV === "production" ? ",{frontmatter:{published:{eq:true}}}" : ""}
    },
    sort:{fields:[frontmatter___date], order:DESC}
    ) {
    edges {
      node {
        id
      }
    }
  }
}
`;

It gives the following error:

Error: BabelPluginGraphQL: Substitutions are not allowed in graphql. fragments. Included fragments should be referenced as ...MyModule_foo.

Steps to reproduce

Build a page with a query that uses a templated string literal.

Expected result

It should first interpret the string literal, and after that make it a graphql fragment

Actual result

It throws an error saying substitution not allowed.

Environment

  • Gatsby version (npm list gatsby): gatsby@1.9.250
  • gatsby-cli version (gatsby --version): 1.1.50
  • Node.js version: v9.11.1
  • Operating System: Linux 4.15.0-2-amd64 #1 SMP Debian 4.15.11-1 (2018-03-20) x86_64 GNU/Linux
@KyleAMathews

This comment has been minimized.

Copy link
Contributor

@KyleAMathews KyleAMathews commented Apr 21, 2018

This sounds like a cool feature but it's not something we support right now. We don't "run" any code in the graphql queries. We just extract the query and then run them. If this is something you want to add support for, I'd love to see an RFC on how the feature would work https://github.com/gatsbyjs/rfcs

@sqrtroot

This comment has been minimized.

Copy link
Author

@sqrtroot sqrtroot commented Apr 21, 2018

Too bad, I'll have a look at writing a RFC for it.
Btw, it does work in gatsby-node.js because graphql is a propper function and it's variables get evaluated before jumping to the function. Maybe this could be a nice pointer as to how to implement this in other files.

@KyleAMathews

This comment has been minimized.

Copy link
Contributor

@KyleAMathews KyleAMathews commented Apr 21, 2018

Yeah, in gatsby-node.js, it is just a function call. For component queries we actually extract the string and then run. We could look into evaluating the string first before running it as a query but we'd need to think about the implications of doing that.

@jquense

This comment has been minimized.

Copy link
Contributor

@jquense jquense commented Apr 21, 2018

babel does offer some amount of static evaluation if interpolations can be determined at compile time. It doesn't help this particular case but it might useful in other cases where you want to store a static value in a variable for using in the query and component. we use it in css-literal-loader for cases like:

const margin = 10;
const height = 50;
const bottom = height + margin;

const styles = css`
  .box {
    height: ${height}px;
    margin-bottom: ${margin}px;
  }

  .footer {
    position: absolute;
    top: ${bottom}px;
  }
`
@sqrtroot

This comment has been minimized.

Copy link
Author

@sqrtroot sqrtroot commented Apr 21, 2018

Okay, I've implemented something in my local version of gatsby:

By changing:

if (quasis.length !== 1) {
throw new Error(
`BabelPluginGraphQL: Substitutions are not allowed in graphql. ` +
`fragments. Included fragments should be referenced ` +
`as \`...MyModule_foo\`.`
)
}
const text = quasis[0].value.raw

To:

let text
if (quasis.length !== 1) {    
  import generate from 'babel-generator'
  text = eval(generate(path.node.quasi).code)
}else{
  text = quasis[0].value.raw
}

it will now generate code for non simple ast's (more than one quasi, almost always indicating some if's) and eval it.

There are some problems with this, for example it will execute the code in context of the babel plugin, not the original code. Variables and functions declared somewhere else in the file will not be compiled and thus not be executed.

I'll write this all up in a RFC, but in the meantime I'll be using this.

@sqrtroot

This comment has been minimized.

Copy link
Author

@sqrtroot sqrtroot commented Apr 21, 2018

@jquense I just tested the constant variable placement, and this unfortunatly also doesn't work.
This is due to the fact that the ast that is generated still contains all the parts of the template literal and not the compiled version of it.
I admit I forgot to add the fact that it isn't evaluated in queries to the title of the post 😜

@jquense

This comment has been minimized.

Copy link
Contributor

@jquense jquense commented Apr 21, 2018

It works for statically determinable values, since all that's needed is the AST, but yeah like i said it doesn't help your case since it's not statically determinable. Generally, I don't think think a feature that requires us to eval the code is good idea, it's never going to be safe, and in most cases would require that the entire tree from that file on is compiled via webpack. You'd need something really advanced and fancy like prepack to do it remotely safely

@sqrtroot

This comment has been minimized.

Copy link
Author

@sqrtroot sqrtroot commented Apr 22, 2018

The current implementation of how queries are parsed doesn't allow the statically determinable values. It would be possible when parsing the whole AST but that's not being done atm.
My hack also doesn't allow for these variables cause it also doesn't generate code for the whole AST. This will still remain a problem and if you want those kind of variables you'll have to rethink the whole strategy of parsing/creating/executing the graphql queries.
I agree that eval is bad. Luckily this will only eval in context of the build process and only on the graphql queries. afaik the only way for them to contain user input is to generate them based on user content/files which in itself might already be a bad idea.

As said before I'll write this all up in a RFC so we can discuss this feature there and I'm going to close this issue since it's not a real issue anymore

@jquense

This comment has been minimized.

Copy link
Contributor

@jquense jquense commented Apr 23, 2018

It would be possible when parsing the whole AST but that's not being done atm

Not quite sure what you mean there, the whole ast is being generated for the file, its a babel plugin, so you really don't have a choice there.

function BabelPluginGraphQL({ types: t }) {
In the one case we may need to explicitly switch to using babel vs babylon (something we are planning on anyway i think)

@sqrtroot

This comment has been minimized.

Copy link
Author

@sqrtroot sqrtroot commented Apr 23, 2018

If i understood the code correctly it only get's information about the tag and not it's dependencies in the AST. So it parses the whole AST, but then only passes one part of it to the graphql handling function. Correct me if i'm wrong.

I Don't know what benefits switching to babel from babylon would provide, but if this would solve some problems like this it would be really nice :)

@Undistraction

This comment has been minimized.

Copy link
Contributor

@Undistraction Undistraction commented May 2, 2018

Given that this isn't possible, can anyone suggest a way of getting a value from a config file into the query?

I have a user config file where a date format can be defined: DD MMMM, YYYY etc.
I need to use this value in my graphql queries. Hard-coded it looks like this:

frontmatter {
  title
  date(formatString: "DD MMMM, YYYY")

I had hoped I could import the config file and interpolate the value, but this doesn't work:

frontmatter {
  title
  date(formatString: "${config.formatString")

I need this string in multiple queries, but at the moment it seems the only way of getting it into the query is via the pathContext, but many of my queries don't use have a pathContext.

@hanoii

This comment has been minimized.

Copy link
Contributor

@hanoii hanoii commented Nov 27, 2018

As mentioned on #10163, I wonder why string interpolation, at least for things like env variables or constant is not simply allowed? It might be unsafer, but much more flexible and after all, a decision of the developer to actively use it.

@jgierer12

This comment has been minimized.

Copy link
Member

@jgierer12 jgierer12 commented Nov 27, 2018

@hanoii it is a technical limitation, it's not simply a switch that can be turned on.

@hanoii

This comment has been minimized.

Copy link
Contributor

@hanoii hanoii commented Nov 27, 2018

@jgierer12 I think that I am trying to imply (with not enough in-depth knowledge so apologize there) is what @KyleAMathews said on #5069 (comment):

We could look into evaluating the string first before running it as a query but we'd need to think about the implications of doing that.

So not saying is a switch, just saying that the above is something that kind of makes sense to me.

@jgierer12

This comment has been minimized.

Copy link
Member

@jgierer12 jgierer12 commented Nov 27, 2018

By the way, I think you can kind of achieve this already using preval.macro codegen.macro: Like any other Babel plugins it is run before extracting queries, so you can do something like this:

import { graphql } from "gatsby"
import codegen from "codegen.macro"

export const query = codegen`
  const query = \`
    query myQuery {
      allMarkdownRemark(
        filter: {
          fileAbsolutePath: { regex: "/.*/blogs/.*/" }
          \${ process.env.NODE_ENV === "production" ? ", { frontmatter: { published: { eq: true } } }" : "" }
        },
      ) {
        edges {
          node {
            id
          }
        }
      }
    }
  \`

  module.exports = "graphql\`" + query + "\`"
`

(Full disclosure: I haven't tested this in any way, there might well be some missing brackets/quotes)

This isn't perfect (no access to variables outside the preval snippet, lots of boilerplate code, no syntax highlighting). But it should work reasonably well as a workaround.

Edit: Actually, preval.macro doesn't work like this. Changed to codegen.macro.

@hanoii

This comment has been minimized.

Copy link
Contributor

@hanoii hanoii commented Nov 27, 2018

@jgierer12 Thanks a lot for this tip. I tried a lot of things with preval and then saw your edit.

I couldn't make it work and I think it's all part of the same thing, in which file-parser takes care of a lot of stuff before even compiling, but I am just diving into this so it might take a bit until I wrap my head properly around all this.

I tried with codegen and I got this:

Error: It appears like Gatsby is misconfigured. Gatsby related `graphql` calls are supposed to only be evaluated at compile time, and then compiled away,. Unfortunately, something went wrong and the query was left in the compiled code.

.Unless your site has a complex or custom babel/Gatsby configuration this is likely a bug in Gatsby.

You seem more versed that I am on this particular thing. If you happen to spare a few mins and can try this out to see if it works it'll help me a lot. It doesn't have to be env or anything, just any other variable. I actually tried it with a static query (the same that worked without it) and it didn't work either.

@jgierer12

This comment has been minimized.

Copy link
Member

@jgierer12 jgierer12 commented Nov 27, 2018

Hmm, it seems like Gatsby doesn't even transform the code before extracting queries.

@KyleAMathews do you know more about this (is Gatsby supposed to run babel transformations before extracting the GraphQL queries)?

@flq

This comment has been minimized.

Copy link

@flq flq commented Mar 9, 2019

so yeah, @jgierer12 showed exactly the use case I would have liked to change that query - so, really, it's just not possible to add a filter when you are in development mode as opposed to production (or vice versa)?

@AAverin

This comment has been minimized.

Copy link

@AAverin AAverin commented Mar 12, 2019

Would be great to have string interpolation.
Very simple example: localized blog index page. I need to query depending on the language that comes in pageContext { locale }

@sqrtroot

This comment has been minimized.

Copy link
Author

@sqrtroot sqrtroot commented Mar 13, 2019

There is a rfc open to create a new tag for this gatsbyjs/rfcs#3. If you want this feature we could discuss workings there and maybe get them to accept the rfc so we can start working on the feature.

@JimLynchCodes

This comment has been minimized.

Copy link
Contributor

@JimLynchCodes JimLynchCodes commented Sep 21, 2019

Hi, I think I am not really understanding the recommended way for adding new images to a gatsby project.

I scaffolded a new project, and the only example image file look like this:

import React from 'react';
import { StaticQuery, graphql } from 'gatsby';
import Img from 'gatsby-image';

/*
 * This component is built using `gatsby-image` to automatically serve optimized
 * images with lazy loading and reduced file sizes. The image is loaded using a
 * `StaticQuery`, which allows us to load the image from directly within this
 * component, rather than having to pass the image data down from pages.
 *
 * For more information, see the docs:
 * - `gatsby-image`: https://gatsby.app/gatsby-image
 * - `StaticQuery`: https://gatsby.app/staticquery
 */

const Image = () => (
  <StaticQuery
    query={graphql`
      query {
        placeholderImage: file(relativePath: { eq: "gatsby-astronaut.png" }) {
          childImageSharp {
            fluid(maxWidth: 300) {
              ...GatsbyImageSharpFluid
            }
          }
        }
      }
    `}
    render={data => <Img fluid={data.placeholderImage.childImageSharp.fluid} />}
  />
);
export default Image;

I wanted to make a generic "Image" component that takes the filename as a parameter:

import React from 'react';
import { StaticQuery, graphql } from 'gatsby';
import Img from 'gatsby-image';

/*
 * This component is built using `gatsby-image` to automatically serve optimized
 * images with lazy loading and reduced file sizes. The image is loaded using a
 * `StaticQuery`, which allows us to load the image from directly within this
 * component, rather than having to pass the image data down from pages.
 *
 * For more information, see the docs:
 * - `gatsby-image`: https://gatsby.app/gatsby-image
 * - `StaticQuery`: https://gatsby.app/staticquery
 */

const Image = (filename) => (
  <StaticQuery
    query={graphql`
      query {
        placeholderImage: file(relativePath: { eq: "${filename ? filename : "gatsby-astronaut.png"}) {
          childImageSharp {
            fluid(maxWidth: 300) {
              ...GatsbyImageSharpFluid
            }
          }
        }
      }
    `}
    render={data => <Img fluid={data.placeholderImage.childImageSharp.fluid} />}
  />
);
export default Image;

but this is giving me the error:

./src/components/image.js
Module build failed (from ./node_modules/gatsby/dist/utils/babel-loader.js):
Error: BabelPluginRemoveGraphQL: String interpolations are not allowed in graphql fragments. Included fragments should be referenced as ...MyModule_foo.

So, I am confused now... I am supposed to make a new js file for every image that I add to my project? Seems very repetitive, but maybe it is necessary for the way the images are loaded? Anyway, some clarification on this would help me out a lot. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.