Lexer, Nodes: Strings with newlines should be output as template literals with unescaped newlines

[amozoss]

When using react relay compiler, Tagged Template Literals have newline characters which causes errors in relay compiler. I've written the sample from the Tagged Template Literal, but included a sample graphql query instead. Would it be possible to compile the literals to not include the new line characters?

Input Code

Sample

upperCaseExpr = (textParts, expressions...) ->
  textParts.reduce (text, textPart, i) ->
    text + expressions[i - 1].toUpperCase() + textPart

greet = (name, adjective) ->
  upperCaseExpr"""
   query HelloQuery {
     allLinks {
       id
       url
     }
   }    
"""

Expected Behavior

Ideally it would compile to the following

upperCaseExpr = function(textParts, ...expressions) {
  return textParts.reduce(function(text, textPart, i) {
    return text + expressions[i - 1].toUpperCase() + textPart;
  });
};

greet = function(name, adjective) {
  return upperCaseExpr`query HelloQuery {
  allLinks {
    id
    url
  }
}    
`;
};

Current Behavior

Instead it is including newline characters

var greet, upperCaseExpr;
upperCaseExpr = function(textParts, ...expressions) {
  return textParts.reduce(function(text, textPart, i) {
    return text + expressions[i - 1].toUpperCase() + textPart;
  });
};

greet = function(name, adjective) {
  return upperCaseExpr`query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}    `;
};

Possible Solution

Not entirely sure, on the surface it seems like the newline characters change the meaning of the Tagged Template Literal. I can put my literal on one line, but thats not ideal for readability of my graphql query.

Context

When running my code through the relay-coffee-compiler I get the error

Module build failed: Syntax Error: Cannot parse the unexpected character "\\".                                                                                                                                                                                                                                                
                                                                                                                                                                                                                                                                                                                              
GraphQL request (1:19)                                                                                                                                                                                                                                                                                                        
1: query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}                                                                                                                                                                                                                                                                  
                     ^                                                                                                                                                                                                                                                                                                                                            

When I only put my coffee template literal on one line, relay-coffee-compiler works. i.e. upperCaseExpr""" query HelloQuery { allLinks { id, url } } """.

Environment

"relay-coffee-compiler": "^0.0.4"

• CoffeeScript version: 2.2.3

[GeoffreyBooth]

@greghuc any thoughts?

[greghuc]

@GeoffreyBooth so I don't have much bandwidth to help at the moment, but I've noted some thoughts below.

First of all, I believe Coffeescript should behave exactly like Javascript in terms of tagged template literals - Coffeescript shouldn't be doing anything clever or different.

I've done a quick experiment in the Chrome 65 console to see how JS template literals handle newlines, and they are treated like any other character. Newline characters are also present in the string presented to the tagged template function, whether there are explicitly supplied (like "I am a\nnew line") or implicitly supplied (

I am a
new line

). So @amozoss, your suggestion that new line characters not be compiled might not make a difference.

Take a look at this (from Chrome console):

`query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}`
"query HelloQuery {
  allLinks {
    id
    url
  }
}"

`query HelloQuery {
  allLinks {
    id
    url
  }
}`
"query HelloQuery {
  allLinks {
    id
    url
  }
}"

=> so the template literal strings look the same whether with explicit or implicit new line characters.

`query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}`.length
52

`query HelloQuery {
  allLinks {
    id
    url
  }
}`.length
52

=> and they have the same number of characters

test = function(textParts, ...expressions) { console.log("textParts: " + JSON.stringify(textParts)); console.log("Expressions: " + JSON.stringify(expressions)) }

test`query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}`
textParts: ["query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}"]
Expressions: []

test`query HelloQuery {
  allLinks {
    id
    url
  }
}`
textParts: ["query HelloQuery {\n  allLinks {\n    id\n    url\n  }\n}"]
Expressions: []

=> and when the two template literal forms are supplied to the tagged template function, the textPart looks the same.

@amozoss, I think you need to do a bit more digging (in Javascript land) to see if you can really find a bug when react relay is supplied tagged template literals with explicit newlines ("new\nline") vs implicit newlines (

new
line

). Because the examples above seem to indicate that there isn't any difference between the two. If you can create a buggy test case, then Coffeescript could take a look at this.

The error from relay-coffee-compiler makes me wonder if there's some form of double escaping issue going on. [Cannot parse the unexpected character "\"] suggests that \n is not being parsed as a single newline character, but as a single slash and a single n.

[GeoffreyBooth]

This is really a string question, not a tagged template literals question. Consider how multiline strings are currently output:

'''
  first line
  second line
'''

'first line\nsecond line';

or

"""
  #{1}
  #{2}
"""

`${1}\n${2}`;

I can see an argument that these should be output as

`first line
second line`;

`${1}
${2}`;

as that would be closer to the user’s original intent. This would also happen to fix the OP’s issue, even while I agree that this issue feels like a downstream bug in relay-coffee-compiler.

[greghuc]

Yeah, so exports.StringWithInterpolations in node.coffee starts generating multiline-strings as multiline template literals, rather than a single-line template literal with newline chars. Would make the generated JS look nicer.

[amozoss]

Thanks for looking into this. Looking at the graphql spec , the way I understand it is the newline characters shouldn't matter.

I dug into relay-compiler (version 1.5.0) a bit (relay-coffee-compiler is just a wrapper of relay-compiler). It uses graphql-js to do the parsing. 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"               

hello/Hello.js

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

It looks like, its escaping the newline chars. 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.

So I also agree it looks like a problem in relay-compiler. I'll report an issue there. Thanks again for digging in.

I do like the idea of matching implicit new lines from coffee to JS. I agree it would make the generated JS look nicer and easier to link back to the coffeescript

[GeoffreyBooth]

The body: 'query HelloQuery {\\n allLinks {\\n id\\n url\\n }\\n}', line implies that it’s escaping backslashes (turning \ into \\) but not taking into account when that backslash was part of an escape sequence (\n) rather than just floating out there on its own (C:\foo).

[amozoss]

Created an issue facebook/relay#2376

[GeoffreyBooth]

@lydell Do you know why the lexer does so much reformatting of strings? I’m looking at functions such as makeDelimitedLiteral and wondering why this logic isn’t happening in nodes.coffee.

I was interested in seeing if I could update StringLiteral or StringWithInterpolations to inspect the value of the string to be output, and if it found any newlines it would output the string as a template literal with real newlines, rather than the current behavior (quoted string with escaped \ns, like you see above). But by the time the string value gets to the node classes, any newline characters have already been escaped, so there’s no way to know whether they were originally “real” newlines or \n escape sequences. Knowing the original string value would also be necessary for eventually supporting Prettier. There’s just so much happening in the lexer I’m hesitant to kick that hornet’s nest and try to start migrating some of those functions into the nodes. cc @zdenko

[lydell]

Do you know why the lexer does so much reformatting of strings?

No, I don’t. It’s always been that way, I think. It’s a good example of CoffeeScript doing too much in the lexer.

[zdenko]

@GeoffreyBooth I did just a quick scan of string parsing in the lexer and came to the same conclusion. Strings should be passed and processed in the nodes.
Next week I'll have more time to take a closer look.

[amozoss]

As a workaround, in relay-coffee-compiler I made the following change, which was good enough for my current use case. Its not ideal though.

index faa759f..ba90840 100644
--- a/src/parser.coffee
+++ b/src/parser.coffee
@@ -22,6 +22,7 @@ parseFile = (baseDir, file) ->
        astDefinitions = []
 
        for template in FindGraphQLTags.memoizedFind(text, baseDir, file, {})
+               template = template.replace /\\n/g, '\n'
                ast = GraphQL.parse(new GraphQL.Source(template, file.relPath))
 
                assert ast.definitions.length,

Thanks for quick responses and the in-depth help.

[zdenko]

@GeoffreyBooth I have prepared a PR which changes how block strings with interpolation are compiled. But, there are few possible breaking changes I think we should discuss first.
Template literal outputs the content without modifying/stripping newlines, indents,...
Example from the console (or Babel):

a = `
  1
    ${s}
      3
`

CS current branch:

a = """
  1
    #{s}
      3
"""

# a = `1\n  ${s}\n    3`

So, the newline at the beginning and end of the string are stripped, and the indent is decreased.

With my PR the output would be changed, i.e. it would be exactly as the example from the console

a = `
  1
    ${s}
      3
`

Consequently, some test currently fails and needs to be adjusted.
For example, this

test "string", ->
  code = """
    do ->
      #{Array(3).join('for [0..0] then\n  ')}
      true
  """

has to be changed to

test "string", ->
  code = """
    do ->
      #{Array(3).join('for [0..0] then\n      ')}
      true
  """

[GeoffreyBooth]

I don't think this is a case where we can have breaking changes. We'll have to do the trimming and other adjustments in the nodes so that the output matches.

[GeoffreyBooth]

@zdenko do you mind posting the branch you had started for this?

[lydell]

The issue here is that all template tags get to choose if they want the “cooked” or “raw” value of the template literal. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#Raw_strings

`one
two`
// equivalent to:
`one\ntwo`

anyTag`one
two`
// not necessarily equivalent to:
anyTag`one\ntwo`

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

Which means that graphql`query {\n foo\n}` is sent as the literal text query {\n foo\n} to the graphql parser (as if JavaScript had no concept of escapes at all).

In other words, it is not safe for CoffeeScript to compile tagged strings like this:

tag"""
one
two
"""

to:

tag`one\ntwo`;

It has to be:

tag`one
two`;

because one cannot know how tag is implemented.

(I stumbled upon this when researching prettier/prettier#4974 and thought I’d stop by and write down my findings).

[GeoffreyBooth]

See #5211

[GeoffreyBooth]

Landed in 2.5.0 🎉