Concatenated template literals are not merged

[qftlzxfz]

With --compilation_level ADVANCED, concatenated template literals are not merged into one.

let url = document.location.href;
console.log(
    `<h1>${url}</h1>`
    + `<p>The URL is ${url}.</p>`
);

url = '//example.com/';
console.log(
    `<h1>${url}</h1>`
    + `<p>The URL is ${url}.</p>`
);

const other_url = '//example.com/';
console.log(
    `<h1>${other_url}</h1>`
    + `<p>The URL is ${other_url}.</p>`
);

Output (with extra line breaks after ;):

let a=document.location.href;
console.log(`<h1>${a}</h1>`+`<p>The URL is ${a}.</p>`);
a="//example.com/";
console.log(`<h1>${a}</h1>`+`<p>The URL is ${a}.</p>`);
console.log("<h1>//example.com/</h1><p>The URL is //example.com/.</p>");

When the values are known, it converts the template string to a normal string and combines them.

I would expect this:

let a=document.location.href;
console.log(`<h1>${a}</h1><p>The URL is ${a}.</p>`);
a="//example.com/";
console.log(`<h1>${a}</h1><p>The URL is ${a}.</p>`);
console.log("<h1>//example.com/</h1><p>The URL is //example.com/.</p>");

Or is there a perhaps a reason not to join the template literals in the first two cases?

[rahulmansharamani14]

Hi @lauraharker, is someone working on it? If not, I'd love to take this.

[lauraharker]

No one is currently working on this, and we'd be happy to review a pull request @rahulmansharamani14. I'll assign you

[rahulmansharamani14]

Hi @lauraharker ! I’ve been investigating this issue and here’s what I’ve found:

What Works: The compiler does merge template literals during + operations in ADVANCED mode when it’s safe to do so. For example:

let g = 'foo';
console.log(`<h1>${g}</h1>` + `<p>${g}</p>`);
// Gets merged into a single template literal

What Fails to Merge: When the interpolated variable contains something like document.location.href, merging is skipped:

let a = document.location.href;
console.log(`<h1>${a}</h1>` + `<p>${a}</p>`);
// These are not merged

Why This Happens: After reading through the compiler internals, I found that the method canBeSideEffected() (in NodeUtil.java) treats all property accesses (GETPROP, GETELEM, etc.) as potentially unsafe:

case GETPROP:
  return true;

This means that expressions like document.location.href are considered to potentially have side effects, even though they are typically just reads.

So even though the wiki Compiler Assumptions states:

“Property reads are assumed to be side effect free…”

…the actual optimization logic is more cautious in implementation — it avoids folding anything unless it can guarantee there are no side effects.

Given that, I had a few follow-up questions:

Is this behavior fully intentional for safety reasons? If both interpolated expressions are referencing the same variable, like:

let a = document.location.href;
console.log(`<h1>${a}</h1>` + `<p>${a}</p>`);

…could this be considered safe to merge?

Happy to propose or contribute a scoped optimization if this feels appropriate :)

[niloc132]

Accessing a property can definitely lead to side effects, but I think not the way you have it phrased. For example, let's say you have this simple object that returns a larger number each time prop is accessed:

var obj = { 
  count: 1,
  get prop() {
    return this.count++;
  }
};

Now, regardless of how you access it, calling obj.prop will result in side effects - but in the context of what you're describing, they don't seem to matter if they are merged?

 console.log(`${obj.prop}, ${obj.prop}`) ;

results in

1, 2

while

 console.log(`${obj.prop},` + ` ${obj.prop}`) ;

results in

3, 4

No matter how you access it in this way, you are causing side effects - but in this context at least, concatenating the strings in order doesn't seem to impact things. Reordering them could though?

var second = ` ${obj.prop}`;
var first = `${obj.prop},`;
console.log(first + second);

6, 5

So if they are concatenated out of order, and there are side effects, then there could be danger here.

On the other hand, if the original code had factored out access to the property, this all becomes moot, as the original access is when the side effects take place, not each later use of the value.

var value = obj.prop;
console.log(`${value},` + ` ${value}`) ;
console.log(`${value}, ${value}`) ;

7, 7
7, 7

...unless the value itself has sideeffects when turning it into a string - I am unsure if that is considered part of this topic or not:

var value2 = {
  toString: function() { return "" + obj.prop; }
}

console.log(`${value2}, ${value2}`)
console.log(`${value2},` + ` ${value2}`)

8, 9
10, 11

This would naturally also have the same issue if the two raw strings were reordered.

[lauraharker]

The compiler also distinguishes between things that "can be side-effected" and things that "potentially have side effects" - sometimes expressions can be both, but what https://github.com/google/closure-compiler/wiki/Compiler-Assumptions refers to is having side effects.

So "can be side-effected" also includes variable reads - given function foo(x) { return 1 ${x} 2; } it's definitely fair to say that the template literal can't have any side effects; but the compiler will still say it may be side-effected because in theory, some code could reassign x. (even within the template literal itself - 1 ${x++} 2 ${x++}`.

I agree with Colin that just concatenating two template literals shouldn't have any dangerous side effects, though.

unless the value itself has sideeffects when turning it into a string

In this case, the compiler already (unsafely) assumes toString never has side effects here.

From looking at PeepholeFoldConstants briefly, it looks like it was originally designed to fold string concatenation, and then sometimes happens to also work for template literals. That's probably why you're seeing this work some of the time. Making it always concatenate template literals sounds reasonable.

The only unit test for that pass involving template literals is just for optimizing values within a template literal interpolation:

test("function foo() {return `${false && y}`}", "function foo() {return `${false}`}");

Adding more unit tests would be a good starting place.

[rahulmansharamani14]

Hi @lauraharker , thanks again for the guidance earlier! I got caught up finishing grad school and haven’t been able to make progress until now. I’m back on it and plan to start by opening a draft PR and adding additional unit tests you suggested.