fix(gatsby): character escape sequences in regex filter in graphql queries

[stefanprobst]

Description

attempt to fix character escape sequences in regex filters, i.e. queries like:

export const query = graphql`
{
  allFile(filter: {base: {regex: "/\\w+\\.PNG$/i"}}) {
    nodes { base }
  }
}
`

this is a follow-up to #25047 which was closed prematurely.

• removes the backslash replaceing from utils/prepare-regex.ts which is not needed (any more? might have been before the switch away from relay)
• removes comment about double-escaping from the docs
• in tests, double-escaping is necessary when query is inline (see e.g. this test here). i have added two query tests in this pr to illustrate this (double-escaping when query defined inline, normal escaping when read via fs.readFile). these are not strictly necessary and could be removed
• with these changes, queries with escape sequences will still break. this is an issue with eslint-plugin-graphql which gatsby uses - throws with "Syntax Error: Invalid character escape sequence`. disabling the plugin here makes things work. i have opened fix: support character escape sequences in template literal apollographql/eslint-plugin-graphql#282 upstream

Related Issues

#25047

[pvdz]

lgtm. Thank you.

Regexes are relatively slow and shouldn't be used if performance is a concern. But oh well, if people want to, they should be able to.

[pvdz]

IIRC this was the actual problem, this regex was missing a global flag?

But I mean if we can drop this entirely that's even better. Will ask @vladar to take a sanity check for graphql.

[paddymccallan]

Double escaping my regex stops the errors, but doesn't return any results the same way the graphiql explorer does... Completely out of ideas.

[pvdz]

(What's the reason this is in Draft mode?)

[stefanprobst]

because it depends on apollographql/eslint-plugin-graphql#282

[vladar]

Some context: escaping was added in #11002 and fixed #10989 and #9944

The change makes sense to me but it is potentially a breaking change. For example, take a blog starter with this query:

{
  allFile(filter:{relativePath: { regex: "/\\\\.png/" }}) {
    nodes {
      relativePath
    }
  }
}

Before this PR it returns a list with a single file. After this PR it returns an empty list (which is what you would expect but it is still a breaking change).

I think we should definitely merge this but maybe as a part of the next major?

[stefanprobst]

the change makes sense to me but it is potentially a breaking change.
I think we should definitely merge this but maybe as a part of the next major?

yes, makes sense.

however, until then we probably should:

• add a global flag to the replace regex here. your example query would currently break with something like regex: "/\\\\w+\\\\.PNG$/i"
• mention here the current double escaping necessity (similar to how it's mentioned here)

sidenote: i believe the original fix in #11002 was just a workaround for the upstream problem in eslint-plugin-graphql -- the error message in both #10989 and #9944 ("Syntax Error: Invalid character escape sequence: \w. graphql/template-strings") actually comes from eslint-plugin-graphql.

[vladar]

Let's get this merged 🚢

[stefanprobst]

• with these changes, queries with escape sequences will still break. this is an issue with eslint-plugin-graphql which gatsby uses - throws with "Syntax Error: Invalid character escape sequence`. disabling the plugin here makes things work. i have opened apollographql/eslint-plugin-graphql#282 upstream

hi - can't remember the exact details - but just a heads up that the upstream issue which this pr depended on is still open (and hasn't received any comments). but maybe it's not necessary anymore?

[vladar]

Thanks for the heads up. I will test this scenario with our latest changes.