Destructuring assignments generate unreachable code

[mweststrate]

The two functions in the typescript snippet below both generate an unreachable var declaration. Although this will work as expected due to hoisting, javascript minifiers might try to remove this dead code and unintentionally introduce global variables. So it would beneficial to move this declarations to the top of the function to eliminate the (valid) warnings.

I tested with the Closure and UglifyJS2 minifiers. Both warn about this declaration but leave it (correctly) in place:

>  uglifyjs c.js -c
WARN: Dropping unreachable code [c.js:5,4]
WARN: Declarations in unreachable code! [c.js:5,4]
WARN: Dropping unreachable code [c.js:15,4]
WARN: Declarations in unreachable code! [c.js:15,4]

Closure:

JSC_UNREACHABLE_CODE: unreachable code at line 5 character 4
    var _a; // unreachable!
    ^
JSC_UNREACHABLE_CODE: unreachable code at line 15 character 4
    var _a; // unreachable!
    ^

c.ts:

export function unreachableVar(a:any[]) {
    const res = this.a.splice(0, 0, ...a);
    return res;
}

export function anotherUnreachableVar(a,b) {
    let c,d;
    switch(a) {
      case true:
        [c,d] = someFunc(b);
    }
    return 3 + c;
}

function someFunc(x) {
    return [1,2]
}

transpiles to c.js:

"use strict";
function unreachableVar(a) {
    var res = (_a = this.a).splice.apply(_a, [0, 0].concat(a));
    return res;
    var _a; // unreachable!
}
exports.unreachableVar = unreachableVar;
function anotherUnreachableVar(a, b) {
    var c, d;
    switch (a) {
        case true:
            _a = someFunc(b), c = _a[0], d = _a[1];
    }
    return 3 + c;
    var _a; // unreachable!
}
exports.anotherUnreachableVar = anotherUnreachableVar;
function someFunc(x) {
    return [1, 2];
}

Tested in tsc 1.7.5 and 1.9.0 (@next)

[mhegazy]

"unreachable" applies to statements, i.e. things that will execute. this is a declaration. var declarations are hoisted to the top of their scope. This is similar to function declaration hoisting, e.g:

function foo () {
   inner();
   return;

   function inner() { // this is not unreachable. the declaration is hoisted to the top of the scope
   }
}

So I would say this is perfectly legal JavaScript, and the checks in uglify/closure are too strict.

[Arnavion]

In case it helps, I've been working around this by moving the declarations to the top using UJS API (I run the TS compiler's output through some fixup steps using UJS, and this is one of them):

// Move var statements at the end of blocks (generated by TS for rest parameters) to the start of the block.
// This is needed to prevent unreachable-code warnings from UJS
root.walk(new UglifyJS.TreeWalker(function (node, descend) {
    if (
        node instanceof UglifyJS.AST_Block &&
        node.body[node.body.length - 1] instanceof UglifyJS.AST_Var
    ) {
        node.body.unshift(node.body.pop());
    }
}));

[mweststrate]

@mhegazy Thanks for the quick response! I am aware the generated code is technically correct. But I think it is nice to produce linter passing / best practice code nonetheless. Just to play nice with other tooling. Especially since typescript is usually combined with post processing tools in serious projects (like uglify, webpack etc etc). Actually I like it that typescript compiled code is a lot less alien than babel compiled code. So I would say keep up the high standard and move the declarations to the top 😉

(By the way, it would be really cool if there was a typescript minifier, which could take into account private fields and such +:100:)

@Arnavion thanks for the work around

[mhegazy]

with #5595 in place, this should be a simple change. but before that, it requires two passes over the tree and a lot of synchronization between the naming logic and the emit logic. in short it will be hard to do and will make the compiler slower if done. #5595 is planned for 2.0, so until then, closing this issue.

[mweststrate]

Sounds great, thanks for looking in to this!

[evmar]

Should we reopen this now that #5595 landed?

[mhegazy]

PRs are welcomed.

[albyrock87]

Seems to be fixed in Typescript 2.9.2 (you can try it in the playground).