Invalid source map generated with multi-line strings about astring HOT 5 CLOSED

The issue is that the map function assumes that code doesn't contain multiple newlines:

https://github.com/davidbonnet/astring/blob/master/src/astring.js#L997

However, it does (astring uses the raw property):

{
  "type": "Literal",
  "value": "var COMPILED=!0,goog=goog||{};goog.global=this;goog.DEBUG=!0;goog.LOCALE=\"en\";goog.provide=function(a){if(!COMPILED){if(goog.isProvided_(a))t",
  "raw": "\"var COMPILED=!0,goog=goog||{};goog.global=this;goog.DEBUG=!0;goo\\\ng.LOCALE=\\\"en\\\";goog.provide=function(a){if(!COMPILED){if(goog.isProvided_(a))t\""
}

A few potential solutions:

1. Use value instead of raw, which lacks the syntactical newlines. This is what Escodegen seems to do.
   • Downside: This will slow down astring, since all strings will need to be JSON'd / escaped properly, resulting in additional string allocations / garbage collection / processing / etc.
   • We could do a search through the string for newlines, and only resort to this when a newline is found. While this is O(literal.length), it doesn't require any string allocations in the common case.
   • Downside 2: This does not match babel's behavior, causing a multiline string test case to fail.
2. Change map to scan for newlines. Rather than just check the end of the string, do a search through the string for newlines.
   • Downside: The astring side (e.g. ignoring source-map operations) of map is currently O(1). This will make it O(code.length).

I think some variant on the first option would be cleanest -- check for newline, special case if found.

from astring.

Thanks @jvilk for reporting this.

Solution 1 is best (O(code.length) will always be worse than O(literal.length)). Inspecting the raw value without JSONing should keep Astring fast.

from astring.

The question then is: Do we use value instead of raw in that case? That behavior outputs the string on a single line, which does not match Babel, which outputs the string as multiple lines.

If you want astring to match Babel, then there needs to be a clean way to write a multiline code segment.

from astring.

No need to match Babel specifically. And counting the occurrences of \\\r?\n should suffice.

from astring.

I've created a PR for this: #354

I went with option 2 to start and establish test cases, since it's a simple and very reliable+correct approach. Though it runs for each character of the code it's practically very fast, about 300 MB/s on my macbook. But I can investigate option 1 as well if we think the cost is high.

Something notable is that simple JSON.stringification in option 1 does not work for template literals. That is, if we see a template literal such as

const a = `hello
world`;

JSON.stringify'ing that value into const a = "hello\nworld" would be a change in the syntax tree. We could manually trim out the quotes for backticks, but that leaves unnecessarily escaped characters in the string, which is ugly imo.

from astring.