Uglifyjs: [ES8] feature request: support trailing commas in function parameter lists and calls

[kzc]

I've encountered a few ES2017/ES8 code bases that are using trailing commas in function parameter lists and calls when testing async/await. Node 8.x and the latest FireFox and Chrome browsers support it. It's reasonable that uglify-es should also support it.

$ echo '!function(a, b, ){ console.log(a + b); }(3, 4, );' | node800
7

Note: 1: Trailing commas in destructuring appears to already be supported by uglify-es:

$ echo 'function f({a = 1, b: c = 2,}) { return a + c; }' | bin/uglifyjs
function f({a:a=1,b:c=2}){return a+c}

$ echo 'f({a = 1, b: c = 2,});' | bin/uglifyjs
f({a:a=1,b:c=2});

Note 2: I don't see any mention of trailing commas in sequences in any article, and it is not supported by node 8.x, nor Chrome:

$ echo 'var seq = (1, 2, 3, );' | node800
[stdin]:1
var seq = (1, 2, 3, );
                    ^
SyntaxError: Unexpected token )

References:

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Trailing_commas
• https://github.com/tc39/proposal-trailing-function-commas
• http://2ality.com/2016/02/ecmascript-2017.html

Official ECMAScript Conformance Test Suite:

• https://github.com/tc39/test262/blob/4fd91d8904cb53fdefa47b52acf2e01e210816eb/test/language/expressions/call/trailing-comma.js
• https://github.com/tc39/test262/blob/4fd91d8904cb53fdefa47b52acf2e01e210816eb/test/language/expressions/function/params-trailing-comma-length.js
• https://github.com/tc39/test262/blob/4fd91d8904cb53fdefa47b52acf2e01e210816eb/test/language/expressions/function/params-trailing-comma.js
• https://github.com/tc39/test262/blob/4fd91d8904cb53fdefa47b52acf2e01e210816eb/test/language/expressions/arrow-function/params-trailing-comma-length.js

[kzc]

A new uglify-es parser option "ecma" can be introduced defaulting to 8 to allow trailing commas in parameter lists and calls.

[alexlamsl]

Having different default values for ecma for different stages is going to make that toplevel shorthand a bit confusing.

[kzc]

It's not intended to be out of sync. Generally the top level ecma would set all stages.

[alexlamsl]

How about parameters in arrow functions?

[kzc]

See last link in top post.

[alexlamsl]

... and how about new foo(a, b,)?

[kzc]

I didn't see an explicit test for it, but let's assume new is also included since node 8.x supports it.

$ node800 -p 'new RegExp("ab+", "g", ).toString()'
/ab+/g

[kzc]

Of the new ES8 features only async/await and optional trailing comma after params/args involve grammar changes. The rest are library-level changes:

http://2ality.com/2016/02/ecmascript-2017.html

Mind you, ES8 has yet to be finalized and that article might be out of date.

[alexlamsl]

Mind you, ES8 has yet to be finalized and that article might be out of date.

I shall rely on you to ping me when things change then :wink:

[kzc]

The notable failing cases according to MDN:

function f(,) {} // SyntaxError: missing formal parameter
(,) => {};       // SyntaxError: expected expression, got ','
f(,)             // SyntaxError: expected expression, got ','

function f(...p,) {} // SyntaxError: parameter after rest parameter
(...p,) => {}        // SyntaxError: expected closing parenthesis, got ','

But a trailing comma after a spread argument is valid in a call:

//  Check that trailing commas are permitted after spread arguments
//  in a call expression.
function foo() {}
foo(...[],);

When in doubt, see what acorn and node 8.x do:

functions with final spread param:

$ echo '(...p,) => {}' | node_modules/.bin/acorn --ecma2017
Comma is not permitted after the rest element (1:5)

$ echo 'function foo(...p,) {}' | node_modules/.bin/acorn --ecma2017
Comma is not permitted after the rest element (1:17)



md5-9f39a948c128e2fe28e499d05efb493d



$ echo '(...p,) => {}' | node800
(...p,) => {}
     ^
SyntaxError: Rest parameter must be last formal parameter



md5-964779b1fbd41a376cd3476e85350b26



$ echo 'foo(...[],);' | node_modules/.bin/acorn --ecma2017
{
  "type": "Program",
  "start": 0,
  "end": 13,
  "body": [
    {
      "type": "ExpressionStatement",
...valid...



md5-9f39a948c128e2fe28e499d05efb493d



$ echo 'console.log(...[1, 2], );' | node800
1 2

[kzc]

It might be obvious (is anything in a spec obvious?) but it's worth mentioning that more than one trailing comma in a function parameter list or call argument list is not valid:

$ echo 'console.log("hello",, );' | node800
[stdin]:1
console.log("hello",, );
                    ^
SyntaxError: Unexpected token ,

$ echo 'function foo(a, b,,) {}' | node800
function foo(a, b,,) {}
                  ^
SyntaxError: Unexpected token ,

[kzc]

I think I've enumerated all the fringe outliers. I think implementing this will take a fraction of the time of researching it.

[alexlamsl]

Ugh... TIL:

$ echo '(a, ...b);' | uglifyjs
a,...b;

$ echo '(a, ...b);' | node
[stdin]:1
(a,...b);
   ^^^

SyntaxError: Unexpected token ...
    at createScript (vm.js:74:10)

[kzc]

Ugh... TIL:

I suspected that might be going on in params_or_seq_ which is why I made the comment about sequences in the top post.

[kzc]

Regarding the implementation time remark - let me qualify that - I meant time without tests. The tests often take 10 times as much time to compose as the implementation.

[alexlamsl]

The tests often take 10 times as much time to compose as the implementation.

Most of the time spent is for me to scroll through this issue and copy your examples as tests :ghost:

[kzc]

"...there are known knowns; there are things we know we know. We also know there are known unknowns; that is to say we know there are some things we do not know. But there are also unknown unknowns – the ones we don't know we don't know."

https://en.wikipedia.org/wiki/There_are_known_knowns

[alexlamsl]

I like to visualise that as a circle - the area within are known knowns, the circumference are the known unknowns. Then you have the infinite abyss all around you, waiting for sense to be made out of.