facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.37k stars 1.82k forks source link

relay-compiler: Does not account for backslashes that are part of escape sequences. #2376

Open amozoss opened 6 years ago

amozoss commented 6 years ago

First off thanks for creating relay, its fabulous.

I'm having an issue related to graphql tagged template literals. I'm including explicit new line characters when forming my queries. hello/Hello.js

import graphql from 'react-relay'
graphql`query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}`

schema.graphql

type Link {
  description: String
  id: ID!
  url: String
}

type Query {
  allLinks: [Link]!
}

Looking at the graphql spec, the way I understand it is the newline characters shouldn't matter.

However, when using relay-compiler 1.5.0, I get the error Parse error: Syntax Error: Cannot parse the unexpected character "\\".

I logged just before the error is thrown with console.log(source) which gave the output: $ ./node_modules/.bin/relay-compiler --src ./hello --schema ./schema.graphql

HINT: pass --watch to keep watching for changes.           
Source {                     
  body: 'query HelloQuery {\\n  allLinks {\\n    id\\n    url\\n  }\\n}',                                             
  name: '/home/dan/projects/myapp5/hello/Hello.js',        
  locationOffset: { line: 2, column: 9 } }                 
ERROR:                       
Parse error: Syntax Error: Cannot parse the unexpected character "\\".                                                

/home/dan/projects/myapp5/hello/Hello.js (2:27)            
2:         query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}                                                  
                             ^                             
 in "Hello.js"               

It looks like, its escaping backslashes, even when the backslash should be considered part of an escape sequence (such as \n). It also seemed, by looking at the lexer source code, that it handled unicode line feed (\u000a) in the parser, but replacing \n with \u000a, had the same error.

Any thoughts?

lydell commented 5 years ago

Did you know that template tags get to choose from the “cooked” and “raw” value of the template literal? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#Raw_strings

This means that whenever you use a template tag you cannot reason about how escapes anymore. You have to read the source code for the template tag (these things tend not to be documented).

The Relay compiler uses the raw value: https://github.com/facebook/relay/blob/78ec175df72f5a13e1061be5dcb9e2aab6255d86/packages/babel-plugin-relay/getValidGraphQLTag.js#L38

Which is why your example doesn’t work. It’s a clash of expectations, really.

Now, in this particular, as far as I understand you use “explicit newlines” as you say because that’s how CoffeeScript compiles your code? Then it is actually CoffeeScript’s fault, as mentioned here: https://github.com/jashkenas/coffeescript/issues/5019#issuecomment-423810585

Still, Relay should probably document how it handles escapes. (As a side note, I suspect that it is impossible to include a literal ` (backtick) character in a string in a graphql query.)

It’s also interesting to note that the gql tag from Apollo uses the cooked value (not the raw one)! https://github.com/apollographql/graphql-tag/blob/ae792b67ef16ae23a0a7a8d78af8b698e8acd7d2/src/index.js#L158

shahsohil25 commented 5 years ago

Hi @amozoss , Did you find any solution for this issue?

amozoss commented 5 years ago

@shahsohil25 I've just been working around it for now by running the bash script below after I install node modules. I'm still on babel-plugin-relay version 1.6.0 so the script may not work with newer versions. I'm also using a forked version of relay-coffee-compiler with these changes https://github.com/amozoss/relay-coffee-compiler/commit/c8bb4810b26c556330e16f4a0a2dfca11120c263

set -e
FILE=./node_modules/babel-plugin-relay/lib/getValidGraphQLTag.js
if [[ $OSTYPE == darwin* ]]; then
  perl -p -i -n -e 's/.*var text =.*/ var text = quasis\[0\].value.raw;text = text.replace(\/\\\\n\/g, "\\n");/g' $FILE
else
  sed -i '/var text = quasis\[0\].value.raw;/a text = text.replace(/\\\\n/g, "\\n");' $FILE
fi

Thanks @lydell for digging even deeper. Looks like the actual issue is in coffeescript.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.