Refactoring JS breaks variable scope

[ianleeder]

Tested using 1.20.6

Describe the bug NUglify refactors JS code, and moves variables inside block scope, breaking functions that reference that variable.

To Reproduce This example uses C#11 raw string literals

using NUglify;

string testJs =
$$"""
document.addEventListener("DOMContentLoaded", () => {
    const body = document.querySelector("body");
    if (!body) {
      return;
    }
    const foo = 'bar'
    doIt();
    function doIt() {
        console.log(foo);
    }
  });
""";

UglifyResult uglified = Uglify.Js(testJs);
Console.WriteLine(uglified.Code);

Minified output or stack trace The minified code:

document.addEventListener("DOMContentLoaded",()=>{function i(){console.log(t)}const n=document.querySelector("body");if(n){const t="bar";i()}})

You can clearly see (once formatted), that the variable t is no longer accessible by the function i:

document.addEventListener("DOMContentLoaded", ()=>{
    function i() {
        console.log(t)
    }
    const n = document.querySelector("body");
    if (n) {
        const t = "bar";
        i()
    }
}
)

Uncaught ReferenceError: t is not defined at i

Excepted output code Variable declaration should not be moved inside the if block.

[trullock]

If you change Const to Var does it still happen? What about Let?

[ianleeder]

Changing the variable to var/let/const doesn't make a difference.

Running the same test without all this code inside an event handler works correctly, eg:

const body = document.querySelector("body");
if (!body) {
  return;
}
var foo = 'bar'
doIt();
function doIt() {
  console.log(foo);
}

Generates

function doIt(){console.log(foo)}const body=document.querySelector("body");if(!body)return;var foo="bar";doIt()

The guard-clause hasn't been inverted: if(!body)return;.

Also it no longer renames variables/functions.

[trullock]

The non-renaming is expected as they are in global scope (and be theoretically be used elsewhere)

Otherwise it does seem like a bug then

[caviyacht]

@trullock , any update on this? This is causing some odd issues that our team needs to constantly work around

[trullock]

Sorry, I don't get any time on this. PRs welcome and I'll merge them