Logical assignment breaks on class static properties

[KyleDavidE]

I suspect this is because the variable flattener doesn't like how previous phases transpile local assignment in a way that creates a temporary variable.

Input:

class MyClass {}
console.log(() => {
  return (MyClass.myField ??= {});
});

with flags:

-O ADVANCED
--js in.js
--language_in ECMASCRIPT_NEXT
--language_out ECMASCRIPT_2020

Output:

class a{}console.log(()=>a.g??{});

Expected output:

// any of
class a{}console.log(()=>a.g??={});
var x;console.log(() => x??={});
var x;console.log(() => x??(x={}));
class a{};console.log(() => a.g??(a.g={}));

[concavelenz]

Here is a clearer example:

class MyClass {}
MyClass.myField = 1
console.log(() => {
  return (MyClass.myField ??= {});
});
console.log(MyClass.myField)

which produces the following output:

class a {
}
var b = 1;
console.log(() => a.g ?? (b = {}));
console.log(b);

which clearly shows that "a.g" was not replaced with "b" as expected.

[concavelenz]

Thanks for the report

[concavelenz]

Before collapse properties this looks like:

 var module$exports$other = {};
 class module$contents$other_MyClass {
 }
 module$contents$other_MyClass.myField = 1;
 console.log(() => {
   let $jscomp$logical$assign$tmpm1171185433$0;
   return ($jscomp$logical$assign$tmpm1171185433$0 = module$contents$other_MyClass).myField ?? ($jscomp$logical$assign$tmpm1171185433$0.myField = {});
 });
 console.log(module$contents$other_MyClass.myField);

Which collapse properties is going to struggle with, I wonder why it isn't an error.

[KyleDavidE]

I don't know if it is of interest but they slightly different syntax of

console.log((x) => {
  var tmp;
  return (tmp = MyClass), tmp.foo || (tmp.foo = x);
});

seems to work just fine, at the cost that if the property access doesn't flatten, the resulting code larger.

[brad4d]

Steve agreed to fix this.
Possibly the best way to do that is to change the transpilation of ??= (etc.) to recognize that the value of MyClass in MyClass.prop ??= {} is effectively a constant, so there is no need to create a temporary variable for it.
That would also generate simpler and shorter code.

[KyleDavidE]

Steve agreed to fix this. Possibly the best way to do that is to change the transpilation of ??= (etc.) to recognize that the value of MyClass in MyClass.prop ??= {} is effectively a constant, so there is no need to create a temporary variable for it. That would also generate simpler and shorter code.

That would be an improvement, but I think it would still break under:

// or some other way of getting a namespace
import * as MyNs from "blah";
MyNs.MyClass.blah ??= {};

[KyleDavidE]

I should flag that this happens even if you are using a language-out that supports logical assignment.

An ideal solution would be to entire remove the transpile for language outs that support it and move it to after optimizations for languages outs that don't, but I recognize that might be difficult to make all the optimization steps tolerate.

[brad4d]

I should flag that this happens even if you are using a language-out that supports logical assignment.

I believe that's because closure-compiler always replaces the complicated assignment operators with simple assignment of the equivalent expression as part of its normalization pass. Doing that makes the code much easier to analyze for optimizations.

[KyleDavidE]

I should flag that this happens even if you are using a language-out that supports logical assignment.

I believe that's because closure-compiler always replaces the complicated assignment operators with simple assignment of the equivalent expression as part of its normalization pass. Doing that makes the code much easier to analyze for optimizations.

That lines up with my understanding. To be clear the identifier case fix you are talking about would be massively helpful for my scenario.

[KyleDavidE]

I will note that it has a pathological edge case:

let A = {
  get huh() {
    A = {};
  },
  set huh(x) {
    console.log("hello");
  },
};
A.huh ??= {};

should log hello, but A.huh ?? (A.huh = {}) would not -- but also if you do anything like this you have it coming.