Three.js: Convert concatenated string arrays to template strings

[greggman]

I started to write a script to try to convert all concatenated string arrays into template strings.

In other words

var foo = [
  "several",
  "lines",
  "of",
  "text",
].join( "\n" );

into

var foo = `
  several
  lines
  of
  text
`;

but then I realized I should ask before wasting several hours.

Should I do this or no?

[donmccurdy]

I think this would be the first ES6 in the library? We don't yet use const/let, classes, etc. The import/export statements don't count, as they're converted automatically by Rollup.

Presumably we'll eventually pull off the bandaid, but not sure what the first step would be, or when.

See https://github.com/mrdoob/three.js/issues/6419

[greggman]

There's already lots of this in the library.

See every file in here

https://github.com/mrdoob/three.js/tree/dev/src/renderers/shaders/ShaderChunk

[donmccurdy]

Oh, that's a recent change I guess. No complaint from me then. :)

[looeee]

Template strings have sneaked their way in over the last couple of months - e.g. #14069 - and there are no performance issues (that I'm aware of) so I vote to go all the way and use them everywhere for multiline strings, especially for shaders.

Related to this, I would also vote to use them to replace script type=shader in the examples.

[takahirox]

I just wanna make clear the purpose of the change because I'm not familiar with template strings. Because the initialization should be faster due to no .join()? Or because it's simpler and more readable? And/or any other benefits?

[looeee]

It's simpler and more readable for sure. I'm not aware of any initialisation benefit either way, but I expect it's negligible.

However, the main benefit I can see is that since #15473 was merged, shader code written like this:

export default /* glsl */`
#ifdef USE_ALPHAMAP
    diffuseColor.a *= texture2D( alphaMap, vUv ).g;
#endif
`;

can have have proper syntax highlighting in at least VSCode and Atom editors with just a little bit of setup.

[greggman]

Okay well here's a PR

[donmccurdy]

Because the initialization should be faster due to no .join()? Or because it's simpler and more readable? And/or any other benefits?

In any case, they're compiled out for compatibility with older browsers. https://github.com/mrdoob/three.js/blob/dev/rollup.config.js#L161-L191