google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.31k stars 1.15k forks source link

`var` is not hoisted to the nearest variable scope #4123

Open bradzacher opened 9 months ago

bradzacher commented 9 months ago

input code:

function foo(arg) {
  switch(arg.type){
      case 'foo':
          var _arg_foo;
          (_arg_foo = arg.foo) === null || _arg_foo === void 0 ? void 0 : _arg_foo.foo();
          break;
      case 'bar':
          var _arg_foo1;
          (_arg_foo1 = arg.foo) === null || _arg_foo1 === void 0 ? void 0 : _arg_foo1.bar();
          break;
      case 'baz':
          var _arg_foo2;
          (_arg_foo2 = arg.foo) === null || _arg_foo2 === void 0 ? void 0 : _arg_foo2.baz();
          break;
  }
}
console.log(foo);

Note: input code is the generated by swc downlevelling optional chaining

[Actual CC output:](https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540formatting%2520pretty_print%250A%252F%252F%2520%2540language_out%2520ECMASCRIPT_2019%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F%252F%2520ADD%2520YOUR%2520CODE%2520HERE%250Afunction%2520foo(arg)%2520%257B%250A%2520%2520switch(arg.type)%257B%250A%2520%2520%2520%2520%2520%2520case%2520'foo'%253A%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520var%2520_arg_foo%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520(_arg_foo%2520%253D%2520arg.foo)%2520%253D%253D%253D%2520null%2520%257C%257C%2520_arg_foo%2520%253D%253D%253D%2520void%25200%2520%253F%2520void%25200%2520%253A%2520_arg_foo.foo()%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520break%253B%250A%2520%2520%2520%2520%2520%2520case%2520'bar'%253A%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520var%2520_arg_foo1%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520(_arg_foo1%2520%253D%2520arg.foo)%2520%253D%253D%253D%2520null%2520%257C%257C%2520_arg_foo1%2520%253D%253D%253D%2520void%25200%2520%253F%2520void%25200%2520%253A%2520_arg_foo1.bar()%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520break%253B%250A%2520%2520%2520%2520%2520%2520case%2520'baz'%253A%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520var%2520_arg_foo2%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520(_arg_foo2%2520%253D%2520arg.foo)%2520%253D%253D%253D%2520null%2520%257C%257C%2520_arg_foo2%2520%253D%253D%253D%2520void%25200%2520%253F%2520void%25200%2520%253A%2520_arg_foo2.baz()%253B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520break%253B%250A%2520%2520%257D%250A%257D%250Aconsole.log(foo)%253B%250A)

console.log(function(a) {
  switch(a.type) {
    case "foo":
      var b;
      null === (b = a.g) || void 0 === b ? void 0 : b.g();
      break;
    case "bar":
      var c;
      null === (c = a.g) || void 0 === c ? void 0 : c.h();
      break;
    case "baz":
      var d;
      null === (d = a.g) || void 0 === d ? void 0 : d.i();
  }
});

Expected CC output:

console.log(function(a) {
  var b,c,d;
  switch(a.type) {
    case "foo":
      null === (b = a.g) || void 0 === b ? void 0 : b.g();
      break;
    case "bar":
      null === (c = a.g) || void 0 === c ? void 0 : c.h();
      break;
    case "baz":
      null === (d = a.g) || void 0 === d ? void 0 : d.i();
  }
});

CC does not hoist the vars to the nearest variable scope, even though (as I understand it) it should be runtime equivalent to do so. If it did hoist the variables then it could collapse them into a single declaration and save the additional var per variable - which would leads to +4b per variable declared within the same variable scope.

bradzacher commented 9 months ago

It's worth noting that currently CC doesn't even reorder variable declarations when they're within the same scope:

Input

function foo(arg) {
  var _arg_foo;
  (_arg_foo = arg.foo) === null || _arg_foo === void 0 ? void 0 : _arg_foo.foo();
  var _arg_foo1;
  (_arg_foo1 = arg.foo) === null || _arg_foo1 === void 0 ? void 0 : _arg_foo1.bar();
  var _arg_foo2;
  (_arg_foo2 = arg.foo) === null || _arg_foo2 === void 0 ? void 0 : _arg_foo2.baz();
}
console.log(foo);

Actual CC Output

console.log(function(a) {
  var b;
  null === (b = a.g) || void 0 === b ? void 0 : b.g();
  var c;
  null === (c = a.g) || void 0 === c ? void 0 : c.h();
  var d;
  null === (d = a.g) || void 0 === d ? void 0 : d.i();
});

Expected CC Output

console.log(function(a) {
  var b,c,d;
  null === (b = a.g) || void 0 === b ? void 0 : b.g();
  null === (c = a.g) || void 0 === c ? void 0 : c.h();
  null === (d = a.g) || void 0 === d ? void 0 : d.i();
});

[cc service playground](https://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClosureCompiler%253D%253D%250A%252F%252F%2520%2540compilation_level%2520ADVANCED_OPTIMIZATIONS%250A%252F%252F%2520%2540output_file_name%2520default.js%250A%252F%252F%2520%2540formatting%2520pretty_print%250A%252F%252F%2520%2540language_out%2520ECMASCRIPT_2019%250A%252F%252F%2520%253D%253D%252FClosureCompiler%253D%253D%250A%250A%252F%252F%2520ADD%2520YOUR%2520CODE%2520HERE%250Afunction%2520foo(arg)%2520%257B%250A%2520%2520var%2520_arg_foo%253B%250A%2520%2520(_arg_foo%2520%253D%2520arg.foo)%2520%253D%253D%253D%2520null%2520%257C%257C%2520_arg_foo%2520%253D%253D%253D%2520void%25200%2520%253F%2520void%25200%2520%253A%2520_arg_foo.foo()%253B%250A%2520%2520var%2520_arg_foo1%253B%250A%2520%2520(_arg_foo1%2520%253D%2520arg.foo)%2520%253D%253D%253D%2520null%2520%257C%257C%2520_arg_foo1%2520%253D%253D%253D%2520void%25200%2520%253F%2520void%25200%2520%253A%2520_arg_foo1.bar()%253B%250A%2520%2520var%2520_arg_foo2%253B%250A%2520%2520(_arg_foo2%2520%253D%2520arg.foo)%2520%253D%253D%253D%2520null%2520%257C%257C%2520_arg_foo2%2520%253D%253D%253D%2520void%25200%2520%253F%2520void%25200%2520%253A%2520_arg_foo2.baz()%253B%250A%257D%250Aconsole.log(foo)%253B%250A)