getters and setters in object literals should be valid in es5 target

[kzc]

$ esbuild --version
0.6.28

$ esbuild three.js --target=es5 --error-limit=1
three.js:42422:13: error: Transforming object literal extensions to the configured target environment is not supported yet
        get total() {
                 ^
1 error reached (disable error limit with --error-limit=0)

$ echo 'console.log({get x(){return 1}, set y(v){}}.x);' | esbuild --target=es5
<stdin>:1:18: error: Transforming object literal extensions to the configured target environment is not supported yet
console.log({get x(){return 1}, set y(v){}}.x);
                  ^
<stdin>:1:37: error: Transforming object literal extensions to the configured target environment is not supported yet
console.log({get x(){return 1}, set y(v){}}.x);
                                     ^
2 errors

According to ES5.1 section 11.1.5 and acorn it's valid ES5...

$ echo 'console.log({get x(){return 1}, set y(v){}}.x);' | acorn --ecma5 --silent && echo ok
ok

fwiw, it was invalid in ES3:

$ echo 'console.log({get x(){return 1}, set y(v){}}.x);' | acorn --ecma3 --silent && echo ok
Unexpected token (1:17)

Possible fix:

--- a/internal/parser/parser.go
+++ b/internal/parser/parser.go
@@ -1149,7 +1149,7 @@ func (p *parser) parseProperty(
        // Parse a method expression
        if p.lexer.Token == lexer.TOpenParen || kind != ast.PropertyNormal ||
                opts.isClass || opts.isAsync || opts.isGenerator {
-               if p.lexer.Token == lexer.TOpenParen {
+               if p.lexer.Token == lexer.TOpenParen && kind != ast.PropertyGet && kind != ast.PropertySet {
                        p.markSyntaxFeature(compat.ObjectExtensions, p.lexer.Range())
                }
                loc := p.lexer.Loc()

with patch:

$ echo 'console.log({get x(){return 1}, set y(v){}}.x);' | esbuild --target=es5
console.log({get x() {
  return 1;
}, set y(v) {
}}.x);

$ esbuild three.js --target=es5 --minify | wc -c
  591195

[kzc]

Somewhat related but not worth making a new issue:

Avoid 2 false failures in make uglify:

--- a/scripts/uglify-tests.js
+++ b/scripts/uglify-tests.js
@@ -104,8 +104,8 @@ async function test_case(service, test) {
     var { js: output } = await service.transform(input_code, {
       // Don't use "minifyWhitespace: true" because esbuild uses some ES6 syntax
       // to make things shorter and uglify can't parse ES6.
-      minifySyntax: true,
-      minifyIdentifiers: true,
+      minify: true,
+      target: "es5",
     });
   } catch (e) {
     const formatError = ({ text, location }) => {

Edit: updated patch

I can appreciate that the bugs related to the use of public as an identifier in non-typescript code and the delete of Infinity and NaN are low priority, but the last make uglify bug seems fairly easy to avoid:

$ echo 'var a;console.log(delete(1,a));' | node
true

$ echo 'var a;console.log(delete(1,a));' | esbuild --minify | node
false

$ echo 'var a;console.log(delete(1,a));' | esbuild --minify
var a;console.log(delete a);

Don't simplify the sequence expression if the parent is delete.

[kzc]

Even though it's known that esbuild doesn't downlevel all ES2015+ constructs to ES5, with the proposed patch above esbuild can keep ES5 code as ES5 once minified. That presently is not possible.

Given a valid ES5 input:

$ cat three.js | acorn --ecma5 --silent && echo ok
ok

--minify with no target specified fails ES5 verification:

$ esbuild three.js --minify | acorn --ecma5 --silent && echo ok
Unexpected character '`' (1:128884)

due to an ES5 string literal being promoted to an ES6 template literal.

--minify --target=es5 with the patch passes ES5 verification:

$ esbuild three.js --minify --target=es5 | acorn --ecma5 --silent && echo ok
ok

[kzc]

@evanw There's a new typeof test failure when the uglify target is updated. It's similar to the delete expression failures that were just resolved in esbuild in some ways.

--- a/Makefile
+++ b/Makefile
@@ -244,8 +244,12 @@ test262: esbuild | demo/test262
 # This runs UglifyJS's test suite through esbuild
 
+clean-uglify:
+       rm -rf github/uglify
+       rm -rf demo/uglify
+
 github/uglify:
        mkdir -p github/uglify
        cd github/uglify && git init && git remote add origin https://github.com/mishoo/uglifyjs.git
-       cd github/uglify && git fetch --depth 1 origin 7a033bb825975a6a729813b2cbe5a722a9047456 && git checkout FETCH_HEAD
+       cd github/uglify && git fetch --depth 1 origin e33c727e8bf7687c827d3e88974ead18af263d12 && git checkout FETCH_HEAD
 
 demo/uglify: | github/uglify

I didn't know how to freshen the uglify branch correctly without creating a new clean-uglify target. Perhaps its commands could be folded into the main clean: target or a new deep-clean: target.

$ make clean-uglify uglify
...
esbuild/demo/uglify/test/compress/sequences.js: issue_4079: !!! failed
---INPUT---
{
    try {
        typeof (0, A);
    } catch (e) {
        console.log("PASS");
    }
}
---EXPECTED STDOUT---
PASS

---ACTUAL STDOUT---

1 failed out of 2035

I didn't try updating the terser tests.

[evanw]

Thanks for calling out all of this stuff by the way :)