ES6 and Object.defineProperties issue

[treblereel]

lets say i have the following class:

class Object3D extends EventDispatcher {

    constructor() {

        super();

        const quaternion = new Quaternion();

        Object.defineProperties( this, {
            quaternion: {
                configurable: true,
                enumerable: true,
                value: quaternion
            }
        } );
      }

       onRotationChange() {
        this.quaternion.setFromEuler(........);
     }
 }

and the following warning:

core/Object3D.js:123:7: WARNING - [JSC_INEXISTENT_PROPERTY] Property quaternion never defined on Object3D
  123|      this.quaternion.setFromEuler();
               ^^^^^^^^^^

I have no idea what i do wrong, maybe i missed something ?

[ctjlewis]

The compiler does not understand that this.quaternion is instantiated in the constructor. If you remove Object.defineProperties(...) and use this.quaternion = quaternion, this warning will go away.

Admittedly, it would be best if the compiler could understand the Object.defineProperties statement.

[treblereel]

@ctjlewis thanks for your clarification, so it means closure ignores Object.defineProperties/Object.defineProperty ? Ok, i am working on three.js and i didn't want to change the code too much but looks like i have no choice to make it closure compatible.

[ctjlewis]

Yeah, I think we have had issues about this before, for this exact library and this exact situation. @lauraharker, @brad4d, does this issue (with the Object.defineProperties conflicting with ThreeJS Quaternion class) also seem familiar? Also, thoughts on patching?

Some Googling gives me:

• 1869

• https://github.com/infusion/Quaternion.js/

[nreid260]

If it's infeasible to update the actual property declaration to be a simple assignment, it would also work to declare the property separately. Something like /** @type {!Quaternion} */ Object3D.prototype.quaternion; would inform the compiler about the property with no runtime effect.

[treblereel]

@nreid260 Thanks for advice, but it not able to do it, coz in my case Quaternion extends Object3D, @type {!Quaternion} needs me to add an export Quaternion in Object3D, that leads to :

 Uncaught ReferenceError: Cannot access 'B' before initialization

:(

[nreid260]

How can Quaternion extend Object3D? Since the constructor of Object3D instantiates Quaternion, that would cause infinite recursion.

More to the point though,/** @type {!Quaternion} */ Object3D.prototype.quaternion; could be added anywhere in the program (even a separate file that just contains property declarations). If you chose to add it in the same file that defines Object3D that would also be fine. Quaternion is only referenced as a type (annotation) here, so this declaration is legal before Quaternion is defined. In compiler terms, this is a "forward reference".

As yet another option, if you're able to patch the code, you could replace the defineProperties call with /** @const {!Quaternion} */ this.quaternion = new Quaternion();. That would be the most idiomatic way to express the property to Closure Compiler. @const would give you very similar compile time guarantees to the ones the code has today by defining the property to be non-writable.

[treblereel]

@nreid260 yeah, i know, it sounds strange, but it's very typical for three.js. Cycle deps everywhere ...

I am about to give up adopting three.js to closure, despite the fact, only 260 warnings left (initially it was ~3000) ...

thanks for your support !

[nreid260]

Depending on what the warnings are, you could just @suppress them. That's our standard approach to rolling out new diagnostics. The code works today, so the warnings likely don't correspond to real problems, and our optimizations generally behave safely (if less effectively) in response to issues.

[nreid260]

Just want to add, the compiler recognizes properties creates by Object.defineProperties during optimizations, but not during typechecking. As such, we believe things are working as intended.

That distinction might feel arbitrary, but optimizations generally require less information about properties (usually just "does this name exist somewhere in the program?") so it's easier to add support. Recognizing them during typechecking is more nuanced.

Additionally, optimization failure can cause runtime behaviour issues, so we need to be very conservative, and often silently cancel optimizations when something might be wrong. In contrast, typechecking failure is a compile-time problem, so we can make it more user visible, and defer to the user (via suppressions or annotations) when we get false positives.

[treblereel]

@nreid260

thanks, it works !

class Test {
    constructor() {
      Object.defineProperty( this, 'isTest', { value: true } );
      Object.defineProperties( this, { isTest2: { value: true } } );
    }
}

so isTest2 can be suppressed, but isTest is always undefined :(

class a{constructor(){Object.defineProperty(this,"isTest",{value:!0});Object.defineProperties(this,{a:{value:!0}})}};