Uglifyjs: [ES6] export incompatible with toplevel compress and mangle option

[kzc]

Top level exported symbols should not be mangled nor dropped by compress.

$ bin/uglifyjs -V
uglify-es 3.0.14

$ cat 2038.js 
let LET = 1;
const CONST = 2;
var VAR = 3;
export {LET, CONST, VAR};



md5-542c49fce0d6572aa0410b90977e642f



$ bin/uglifyjs 2038.js -m toplevel
let a=1;const c=2;var n=3;export{LET,CONST,VAR};



md5-542c49fce0d6572aa0410b90977e642f



$ bin/uglifyjs 2038.js -c toplevel
export{LET,CONST,VAR};



md5-542c49fce0d6572aa0410b90977e642f



$ bin/uglifyjs 2038.js --toplevel -c -m
export{LET,CONST,VAR};



md5-8e7987d8d6bb394fe43f22d3e9cc6830



    {
      "_class": "AST_Export",
      "exported_names": [
        {
          "_class": "AST_NameImport",
          "foreign_name": {
            "_class": "AST_SymbolImportForeign",
            "name": "LET"
          },
          "name": {
            "_class": "AST_SymbolImport",
            "name": "LET"
          }
        },

[kzc]

I think there should be a new AST node type named AST_SymbolExport which should be derived from AST_SymbolRef.

Having the new node types AST_NameExport and AST_SymbolExportForeign might be a good idea as well.

[kzc]

Other export issues:

$ bin/uglifyjs -V
uglify-es 3.0.18

$ echo 'export var V = 1;' | bin/uglifyjs -c toplevel
export;

$ echo 'export let L = 2;' | bin/uglifyjs -c toplevel
export;

$ echo 'export const C = 3;' | bin/uglifyjs -c toplevel
export;

[alexlamsl]

If you mean AST_SymbolExport for things like export var V = 1; then it should probably descend from AST_SymbolDeclaration instead of AST_SymbolRef

[kzc]

The two export issues are only superficially related.

Yes, export (var|const|let) name = ... could use AST_SymbolDeclaration.

AST_SymbolRef would be for export {LET,CONST,VAR}; - a user of symbols. That would solve its problem. The use of AST_*Import* data structures doesn't work for exports.

[kzc]

Yes, export (var|const|let) name = ... could use AST_SymbolDeclaration.

This is fine actually - AST_SymbolLet and AST_SymbolConst are already derived from AST_SymbolDeclaration:

        AST_SymbolDeclaration (init) "A declaration symbol (symbol in var/const, function name or argument, symbol in catch)" {
            AST_SymbolVar "Symbol defining a variable" {
                AST_SymbolFunarg "Symbol naming a function argument"
            }
            AST_SymbolBlockDeclaration "Base class for block-scoped declaration symbols" {
                AST_SymbolConst "A constant declaration"
                AST_SymbolLet "A block-scoped `let` declaration"

Just a matter of fixing figure_out_scope() and not dropping the definition in -c toplevel:

$ echo 'export let L = 2;' | bin/uglifyjs -o ast
{
  "_class": "AST_Toplevel",
  "body": [
    {
      "_class": "AST_Export",
      "exported_definition": {
        "_class": "AST_Let",
        "definitions": [
          {
            "_class": "AST_VarDef",
            "name": {
              "_class": "AST_SymbolLet",
              "name": "L"
            },
            "value": {
              "_class": "AST_Number",
              "value": 2
            }
          }
        ]
      }
    }
  ]
}

[alexlamsl]

So I thought I'd look at this issue from the top, and the first hurdle I run into is this:

$ cat 2038.js 
let LET = 1;
const CONST = 2;
var VAR = 3;
export {LET, CONST, VAR};

$ uglifyjs 2038.js -o ast
{
  "_class": "AST_Toplevel",
  "body": [
    {
      "_class": "AST_Let",
      "definitions": [
        {
          "_class": "AST_VarDef",
          "name": {
            "_class": "AST_SymbolLet",
            "name": "LET"
          },
          "value": {
            "_class": "AST_Number",
            "value": 1
          }
        }
      ]
    },
    {
      "_class": "AST_Const",
      "definitions": [
        {
          "_class": "AST_VarDef",
          "name": {
            "_class": "AST_SymbolConst",
            "name": "CONST"
          },
          "value": {
            "_class": "AST_Number",
            "value": 2
          }
        }
      ]
    },
    {
      "_class": "AST_Var",
      "definitions": [
        {
          "_class": "AST_VarDef",
          "name": {
            "_class": "AST_SymbolVar",
            "name": "VAR"
          },
          "value": {
            "_class": "AST_Number",
            "value": 3
          }
        }
      ]
    },
    {
      "_class": "AST_Export",
      "exported_names": [
        {
          "_class": "AST_NameImport",
          "foreign_name": {
            "_class": "AST_SymbolImportForeign",
            "name": "LET"
          },
          "name": {
            "_class": "AST_SymbolImport",
            "name": "LET"
          }
        },
        {
          "_class": "AST_NameImport",
          "foreign_name": {
            "_class": "AST_SymbolImportForeign",
            "name": "CONST"
          },
          "name": {
            "_class": "AST_SymbolImport",
            "name": "CONST"
          }
        },
        {
          "_class": "AST_NameImport",
          "foreign_name": {
            "_class": "AST_SymbolImportForeign",
            "name": "VAR"
          },
          "name": {
            "_class": "AST_SymbolImport",
            "name": "VAR"
          }
        }
      ]
    },
    {
      "_class": "AST_EmptyStatement"
    }
  ]
}

Why is AST_SymbolImport a descendant of AST_SymbolDeclaration as opposed to AST_SymbolRef?

[alexlamsl]

... or did I just repeat what you said in https://github.com/mishoo/UglifyJS2/issues/2038#issuecomment-305242860 ? :sweat_smile:

[kzc]

Why is AST_SymbolImport a descendant of AST_SymbolDeclaration as opposed to AST_SymbolRef?

For import it may be fine. For export it is not.

We shouldn't overload import classes for use with export.

.. or did I just repeat what you said in #2038 (comment) ?

Yep.

[kzc]

The export aliasing is also broken for toplevel mangle:

$ echo 'let x = 1, y = 2; export { x as X, y as Y };' | bin/uglifyjs -cm toplevel
let a=1,b=2;export{x as X,y as Y};

...which makes sense since they're not AST_SymbolRefs.

As for:

$ echo 'export let L = 2;' | bin/uglifyjs -c toplevel
export;

We'd have to exclude removing "unused" toplevel export declarations from -c unused:

$ echo 'export let L = 2;' | bin/uglifyjs -c toplevel,unused=0
export let L=2;

[kzc]

To further complicate things, I just learned that the following is valid ES6 according to both acorn and babel:

$ echo 'export let x = 1, y = 2;' | babel

"use strict";
Object.defineProperty(exports, "__esModule", {
  value: true
});
var x = 1,
    y = 2;
exports.x = x;
exports.y = y;

I mistakenly thought only a single variable definition could be exported - clearly not the case.

By the way, I find it useful to run ES6 imports/export through babel@5 to see what's going on in ES5 language I can understand:

$ echo 'let x = 1, y = 2; export { x as X, y as Y };' | babel | uglifyjs -b

"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
var x = 1, y = 2;
exports.X = x;
exports.Y = y;

[alexlamsl]

--- a/lib/compress.js
+++ b/lib/compress.js
@@ -2099,11 +2099,12 @@ merge(Compressor.prototype, {
                     return true; // don't go in nested scopes
                 }
                 if (node instanceof AST_Definitions && scope === self) {
+                    var in_export = tw.parent() instanceof AST_Export;
                     node.definitions.forEach(function(def){
                         if (def.name instanceof AST_SymbolVar) {
                             var_defs_by_id.add(def.name.definition().id, def);
                         }
-                        if (!drop_vars) {
+                        if (!drop_vars || in_export) {
                             def.name.walk(new TreeWalker(function(node) {
                                 if (node instanceof AST_SymbolDeclaration) {
                                     var def = node.definition();

$ echo 'export var V = 1;' | bin/uglifyjs -c toplevel
export var V=1;

$ echo 'export let L = 2;' | bin/uglifyjs -c toplevel
export let L=2;

$ echo 'export const C = 3;' | bin/uglifyjs -c toplevel
export const C=3;

[kzc]

2124 is related to this issue in so far as it limits the types of allowable import/export ASTs compress and mangle have to consider.