Njs: Template literal support.

[xeioex]

Link.

to convert ugly code like:

r.error(": " + errorset.error + ", " + errorset.error_description); 

into

req.error(`: ${errorset.error}, ${errorset.error_description}`); 

Scope:
1) Multi-line strings
2) Simple variables references ${errorset.error}

Optionally:
3) expression interpolation ${a + b} (not sure)

Not included:
everything else.

Implementation details:
I suggest to see this as a syntax sugar for string concatenations.
${errorset.error}, ${errorset.error_description} =>
String.prototype.concat("",errorset.error, ", ", errorset.error_description)

[hongzhidao]

@xeioex

I plan to implement it next week.

[xeioex]

@hongzhidao

Regarding the scope of work. See the updated initial message.

[drsm]

@xeioex @hongzhidao

expression interpolation ${a + b} (not sure)

I'm using this in node very often.

maybe something like this:

var logmsg = `[${(new Date()).toISOString()}] the message contents ${something} ${one + two}`;

can be transformed to:

var __tmp = Array(6);
__tmp[0] = '[';
__tmp[1] = (new Date()).toISOString();
__tmp[2] = '] the message contents ';
__tmp[3] = something;
__tmp[4] = ' ';
__tmp[5] = one + two;
var logmsg = String.prototype.concat.apply('', __tmp);

or generate an array literal instead of a variable

[hongzhidao]

@xeioex @drsm

Take a look, the patch is only for discussion.
https://gist.github.com/hongzhidao/1bed25550834947a40e79ac6c35cb467

var something = "test something";
var one = 1;
var two = 2;

var logmsg = `[${(new Date()).toISOString()}] the message contents ${something} ${one + two}`;
console.log(logmsg);

'[2019-03-26T04:09:57.288Z] the message contents test something 3'

[drsm]

@hongzhidao

>> var s = `1${ s }`
undefined
>> s
'11'

vs.

> var s = `1${ s }`
undefined
> s
'1undefined'

also:

var s = '0';
s = `x${s += '1'}`;

[hongzhidao]

@hongzhidao

>> var s = `1${ s }`
undefined
>> s
'11'

vs.

> var s = `1${ s }`
undefined
> s
'1undefined'

also:

var s = 0;
s = `x${s += '1'}`;

Get it, thanks.

[drsm]

@hongzhidao
here is another one:

>> var o = 1; `o = \`${o}\``
SyntaxError: Unexpected token "$" in shell:1
// expected: 'o = `1`'

[hongzhidao]

@drsm

I haven't considered \, I want to confirm the way of parser and generator first with you and @xeioex .
But your test is really useful.

[xeioex]

@hongzhidao

I want to confirm the way of parser and generator

Overall parser looks OK.
regarding generator, I like more drsm's way.
Because, using simple addition would produce too much temporary values.

`${a} ${b} ${c} ${d}`
=> 'a ', 'a b', 'a b ', 'a b c', 'a b c ', 'a b c d'

[drsm]

@hongzhidao
the functionality looks promising, thanks!
some tests that pass:

`x${`y${'z'}`}`
`${{ toString: function() { throw new Error('err'); } }}`

todo:

> var s = `${s='a'}${s}`; s
'aa'
> var s = `${s}${s='a'}`; s
'undefineda'

>> `${`
SyntaxError: Unterminated template literal "`" in shell:1
>> `${;}`
SyntaxError: Unexpected token ";" in shell:1
// Missing } in template expression

[hongzhidao]

@drsm

var __tmp = Array(6);
__tmp[0] = '[';
__tmp[1] = (new Date()).toISOString();
__tmp[2] = '] the message contents ';
__tmp[3] = something;
__tmp[4] = ' ';
__tmp[5] = one + two;
var logmsg = String.prototype.concat.apply('', __tmp);
or generate an array literal instead of a variable

generate an array literal instead of a variable
What does it mean? Show example please.

[drsm]

@hongzhidao

console.log(
    String.prototype.concat.apply('', [
        '[',
        (new Date()).toISOString(),
        '] the message contents ',
        'something',
        ' ',
        1 + 2
    ])
);

[xeioex]

@hongzhidao

What does it mean? Show example please.

Igor suggest to add a new VM instructions for invoking String.prototype.concat.apply().

So genetated code can look like:

PROPERTY SET
PROPERTY SET
...
TEMPLATE LITERAL // to be added

[hongzhidao]

Igor suggest to add a new VM instructions for invoking String.prototype.concat.apply().

Totally argee.

[hongzhidao]

@xeioex @drsm

This feature supports two formats:

1. template literal

var logmsg = `${start}[${(new Date()).toISOString()}] the message contents ${something} ${one + two}`;
// can be transformed to:
var __tmp = Array(9);
__tmp[0] = '';
__tmp[1] = start;
__tmp[2] = '[';
__tmp[3] = (new Date()).toISOString();
__tmp[4] = '] the message contents ';
__tmp[5] = something;
__tmp[6] = ' ';
__tmp[7] = one + two;
__tmp[8] = '';

PROPERTY SET
PROPERTY SET
...
TEMPLATE LITERAL // to be added



md5-44d76423415551a670bca75c2b700ba5



function foo() { console.log(arguments); }

foo`${start}[${(new Date()).toISOString()}] the message contents ${something} ${one + two}`;
// can be transformed to:
var __tmp = Array(5);
__tmp[0] = Array(5);
__tmp[0][0] = '';
__tmp[0][1] = '[';
__tmp[0][2] = '] the message contents ';
__tmp[0][3] = ' ';
__tmp[0][4] = ''
__tmp[1] = start;
__tmp[2] = (new Date()).toISOString();
__tmp[3] = something;
__tmp[4] = one + two;
foo(__tmp);



md5-26aaeeed0ca8227713204312d7f2dc08



function foo() {console.log(arguments)}

var start = 'NJS:';
var something = 'test';
var one = 1;
var two = 2;

foo`${start}[${(new Date()).toISOString()}] the message contents ${something} ${one + two
}`;



md5-d01b19c978de860a98826dab03922900



{0:['','[','] the message contents ',' ',''],1:'NJS:',2:'2019-03-31T07:17:17.844Z',3:'test',4:3}

Welcome to suggest, I plan to refactor njs_parser_array() (maybe include njs_parser_object()) and njs_parser_call_expression() first.
Better naming is highly welcome.

@xeioex
https://gist.github.com/hongzhidao/d9bc57eb1f9ea1fccac8ac1b3b65a470#file-template-literal-03-31-patch-L731
Can we simplify the arguments? I don't like the design of **parent and *index but can't find a better way yet.

BTW, do you think it's time to introduce njs_parser_terminator.c?

1. njs_parser.c: parse statements and basic functions about parser.
2. njs_parser_terminator.c: parse njs_parser_object, njs_parser_array, njs_parser_reference, etc.
3. njs_parser_expression.c: parse expression.

[xeioex]

@hongzhidao

BTW, do you think it's time to introduce njs_parser_terminator.c?

Agree.

Plans:
Implement the second format tagged template first, since it needn't add new instructions.
And njs_parser_array and njs_parser_call_expression will be refactored.
Implement the format of template literal.

Agree. Looks promising.

Can we simplify the arguments? I don't like the design of **parent and *index but can't find a better way yet.

Cannot suggest anything better now, will look into later.

[drsm]

deleted

[hongzhidao]

@xeioex

Refactored out njs_parser_terminator.c.
https://gist.github.com/hongzhidao/4c02ddf28ae77ec4bca2f75bc35e8ba3

Introduced njs_variable_copy().
https://gist.github.com/hongzhidao/f5174774af6c8d2256888ffb650c5429

[xeioex]

@hongzhidao

Refactored out njs_parser_terminator.c.
https://gist.github.com/hongzhidao/4c02ddf28ae77ec4bca2f75bc35e8ba3

I like the change, except, I think njs_parser_terminal.c should be a bit better. Will do.

[hongzhidao]

@xeioex

Improved njs_parser_object() and njs_parser_array().
https://gist.github.com/hongzhidao/38c5208ba0dcbcf92b08ae267841bb8a

There is one place I still want to improve, but I can't find a better way.

1. According to njs_parser_object(), the object is created outside for loop. Is this right?

diff -r 03be823cd95b njs/njs_parser.c
--- a/njs/njs_parser.c  Tue Feb 12 18:56:04 2019 +0300
+++ b/njs/njs_parser.c  Mon Apr 01 11:18:31 2019 +0800
@@ -2352,6 +2352,13 @@ njs_parser_array(njs_vm_t *vm, njs_parse
     index = 0;
     left = NULL;

+    object = njs_parser_node_new(vm, parser, NJS_TOKEN_OBJECT_VALUE);
+    if (nxt_slow_path(object == NULL)) {
+        return NJS_TOKEN_ERROR;
+    }
+
+    object->u.object = obj;
+
     for ( ;; ) {
         token = njs_parser_token(parser);
         if (nxt_slow_path(token <= NJS_TOKEN_ILLEGAL)) {
@@ -2378,13 +2385,6 @@ njs_parser_array(njs_vm_t *vm, njs_parse
         node->u.value.data.truth = (index != 0);
         index++;

-        object = njs_parser_node_new(vm, parser, NJS_TOKEN_OBJECT_VALUE);
-        if (nxt_slow_path(object == NULL)) {
-            return NJS_TOKEN_ERROR;
-        }
-
-        object->u.object = obj;
-
         propref = njs_parser_node_new(vm, parser, NJS_TOKEN_PROPERTY);
         if (nxt_slow_path(propref == NULL)) {
             return NJS_TOKEN_ERROR;

1. simplify njs_parser_array_item.
   The function will be used in template literal. I still want to eliminate the parameter object, but are not able to archive it. Unless we move object inside for loop.

njs_parser_array_item(njs_vm_t *vm, njs_parser_t *parser,
    njs_parser_node_t *array, njs_parser_node_t *object,
    njs_parser_node_t *value)

[xeioex]

@hongzhidao

Introduced njs_variable_copy().
https://gist.github.com/hongzhidao/f5174774af6c8d2256888ffb650c5429

BTW, do you have plans to reuse it somewhere else? If not, I am not sure it is worth it.

[hongzhidao]

@hongzhidao

Introduced njs_variable_copy().
https://gist.github.com/hongzhidao/f5174774af6c8d2256888ffb650c5429

BTW, do you have plans to reuse it somewhere else? If not, I am not sure it is worth it.

No plan, I only want to make njs_parser.c clean, now it's too large :), I also agree to keep unchanged.

[hongzhidao]

@xeioex Take a look again.

Here's the patch of supporting initial tagged template.
https://gist.github.com/hongzhidao/b5ca18e7ab87dc5dd1199867ae057981

But the goal is to refactor njs_parser_object/array and njs_parser_call_expression.
I plan to split this into two patches.

1. Refactored njs_parser_object() and njs_parser_object().
2. Refactored njs_parser_call_expression().

simplify njs_parser_array_item.

https://gist.github.com/hongzhidao/b5ca18e7ab87dc5dd1199867ae057981#file-njs-04-01-patch-L459
I try it in this way.

Can we simplify the arguments? I don't like the design of **parent and *index but can't find a better way yet.
Cannot suggest anything better now, will look into later.

https://gist.github.com/hongzhidao/b5ca18e7ab87dc5dd1199867ae057981#file-njs-04-01-patch-L285
Keep it simple.

[xeioex]

@hongzhidao

Refactored njs_parser_object() and njs_parser_object().
Refactored njs_parser_call_expression().

Looks good. Waiting for separate patches.

[hongzhidao]

@xeioex

1. Refactored njs_parser_object() and njs_parser_object().
2. Refactored njs_parser_call_expression().
   https://gist.github.com/hongzhidao/2deb9265753f3b8738d3dee5ace277fb

One detail. Is it right?

diff -r 03be823cd95b njs/njs_parser_expression.c
--- a/njs/njs_parser_expression.c   Tue Feb 12 18:56:04 2019 +0300
+++ b/njs/njs_parser_expression.c   Mon Apr 01 21:50:49 2019 +0800
@@ -815,7 +815,6 @@ njs_parser_call_expression(njs_vm_t *vm,
         case NJS_TOKEN_NAME:
             func = node;
             func->token = NJS_TOKEN_FUNCTION_CALL;
-            func->scope = parser->scope;  // I removed it.

             break;

[hongzhidao]

deleted.

[hongzhidao]

@xeioex

Here's the patch.
https://gist.github.com/hongzhidao/f72f9023dfebc8f751b4745b3965d0d9

Help check the design. I'll continue to perfect details.

@drsm Welcome to test.

BTW, \ has not been supported yet.

@xeioex In template literal, string should behave like njs_lexer_string, right?
consider String.raw() together.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/raw

[xeioex]

@hongzhidao

https://gist.github.com/hongzhidao/f72f9023dfebc8f751b4745b3965d0d9

The last (4th) revision looks good to me.

String.raw()

looks interesting.

[drsm]

@xeioex @hongzhidao
there also a raw array for tagged literals:

$ node
> `\unicode`
`\unicode`
 ^^^^^^

SyntaxError: Invalid Unicode escape sequence

> String.raw`\unicode`
'\\unicode'
> function test(str, ...rest) { console.log(str); }
undefined
> test`\unicode`
[ undefined ]
undefined
> function test(str, ...rest) { console.log(str, str.raw); }
undefined
> test`\unicode`
[ undefined ] [ '\\unicode' ]
undefined

[hongzhidao]

@xeioex @drsm

Take a look. (including escape string now)
https://gist.github.com/hongzhidao/f72f9023dfebc8f751b4745b3965d0d9

BTW, String.raw has not been supported, I'm not sure how to implement it yet.
Is it used often?

@drsm welcome to test, I think this feature is ready now :)

[drsm]

@hongzhidao

@drsm welcome to test, I think this feature is ready now :)

works fine for me, thanks!

BTW, String.raw has not been supported, I'm not sure how to implement it yet.
Is it used often?

it's just a syntax sugar, and it will complicate things (included in ES2018).

[hongzhidao]

it's just a syntax sugar, and it will complicate things (included in ES2018).

Agree, do you think it's worth to implement String.raw() now?

[drsm]

@hongzhidao
I'm OK if there will be a separate issue for that.

[xeioex]

@hongzhidao @drsm
Great job guys! Many thanks to both of you. Template literals are really useful for njs in nginx. Without your contribution would still be on the todo list.