What to do about static get in es6 classes

[zepumph]

I see 10 usages of static get in our code (Input.js and Client.js). Likely all from me. After talking with @samreid about it, and looking at phetsims/phet-info#82 (comment) and phetsims/wave-interference#281 it seems like we don't want to put attributes on classes this way. @samreid said it has to do with babel.

Should these usages be converted? Do we allow static getters?

[samreid]

Right, those look like good candidates to move to static properties as described in phetsims/wave-interference#281. Once babel supports static attributes, they can be moved back.

[zepumph]

Sounds good I'll convert usages.

[zepumph]

It seems like this should be added to a lint rule, perhaps " static get " is now bad text in sims. Would you like me to add this @samreid?

[samreid]

Yes please.

[zepumph]

I can start with this patch next time I work on it:

Index: common/js/Client.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- common/js/Client.js	(revision af8302605c74041a4f7b25f846a365e6e6d3a545)
+++ common/js/Client.js	(date 1547841007119)
@@ -997,59 +997,6 @@
       this.frame.src = src;
     }
 
-    /**
-     * Private to keep out of the client facing jsdoc generation. Publicly used in our own wrappers to support all runs.
-     * Whether or not the wrapper has been built. If the wrapper is built, then the template variable will be filled in,
-     * so the curly brace will not be the first character.
-     * @type {boolean}
-     * @private - Private to keep out of the client facing jsdoc generation, but this is public within the PhET-iO wrapper suite.
-     */
-    static get IS_BUILT() { return IS_BUILT; }
-
-
-    /**
-     * The hyphenated name of the simulation, e.g. "simulation-name"
-     * @type {string}
-     * @public
-     * @constant
-     */
-    static get SIMULATION_NAME() {return simulationName;}
-
-    /**
-     * The name of the simulation as a title string, e.g. "Simulation Name"
-     * @type {string}
-     * @public
-     * @constant
-     */
-    static get SIMULATION_DISPLAY_NAME() {return SIMULATION_DISPLAY_NAME;}
-
-    // Public jsdoc below notes only the form of the production version to make it simpler to clients.
-    /**
-     * The full version of the simulation, in the form of "MAJOR.MINOR.MAINTENANCE".
-     * @type {string}
-     * @public
-     * @constant
-     */
-    static get SIMULATION_VERSION() {return SIMULATION_VERSION;}
-
-    // This will be null in requirejs mode
-    /**
-     * The path where the lib file has been deployed. This path uses the "MAJOR.MINOR" version to get automatic bugfixes.
-     * @type {string}
-     * @public
-     * @constant
-     */
-    static get PHET_IO_LIB_ABSOLUTE_PATH() {return phetioLibAbsolutePath;}
-
-    /**
-     * The name of the simulation in lowercase camelCasing
-     * @type {string}
-     * @public
-     * @constant
-     */
-    static get CAMEL_CASE_SIMULATION_NAME() {return toCamelCase();}
-
-
     /**
      * Appends a query string to a given url.
      * @param {string} url - may or may not already have other query parameters
@@ -1126,6 +1073,60 @@
     }
   };
 
+
+  /**
+   * Private to keep out of the client facing jsdoc generation. Publicly used in our own wrappers to support all runs.
+   * Whether or not the wrapper has been built. If the wrapper is built, then the template variable will be filled in,
+   * so the curly brace will not be the first character.
+   * @type {boolean}
+   * @private - Private to keep out of the client facing jsdoc generation, but this is public within the PhET-iO wrapper suite.
+   */
+  window.phetio.Client.IS_BUILT = IS_BUILT;
+
+
+  /**
+   * The hyphenated name of the simulation, e.g. "simulation-name"
+   * @type {string}
+   * @public
+   * @constant
+   */
+  window.phetio.Client.SIMULATION_NAME = simulationName;
+
+  /**
+   * The name of the simulation as a title string, e.g. "Simulation Name"
+   * @type {string}
+   * @public
+   * @constant
+   */
+  window.phetio.Client.SIMULATION_DISPLAY_NAME = SIMULATION_DISPLAY_NAME;
+
+  // Public jsdoc below notes only the form of the production version to make it simpler to clients.
+  /**
+   * The full version of the simulation, in the form of "MAJOR.MINOR.MAINTENANCE".
+   * @type {string}
+   * @public
+   * @constant
+   */
+  window.phetio.Client.SIMULATION_VERSION = SIMULATION_VERSION;
+
+  // This will be null in requirejs mode
+  /**
+   * The path where the lib file has been deployed. This path uses the "MAJOR.MINOR" version to get automatic bugfixes.
+   * @type {string}
+   * @public
+   * @constant
+   */
+  window.phetio.Client.PHET_IO_LIB_ABSOLUTE_PATH = phetioLibAbsolutePath;
+
+  /**
+   * The name of the simulation in lowercase camelCasing
+   * @type {string}
+   * @public
+   * @constant
+   */
+  window.phetio.Client.CAMEL_CASE_SIMULATION_NAME = toCamelCase();
+
+
   const DEFAULT_CLIENT_NAME = 'Client';
 
   // As of this writing, IE doesn't support currentScript, which is only currently used in multi and diff wrappers.

[zepumph]

Implemented above, and the lint rule was added to the sim specific bad text. @samreid please review.

[samreid]

Looks great to me, my only question was about the leading whitespace before 'static'. Does eliminating the whitespace introduce any false positives? The leading whitespace seems to be relying on our nesting and formatting conventions.

[zepumph]

No false positives or anything. I thought about it, but it seems the most complete to me. Also note that I'm pretty sure this will only ever be within a class declaration and scope. What do you think is best? I don't want a false positive on "what an ecstatic get he is." :)

[samreid]

That sounds reasonable, thanks for adding this! Closing.

[jonathanolson]

It seems useful to have static get, AND it doesn't seem like we're transpiling classes now. Can we have this? Display's get focus turns into a static get when moving it to class.

[zepumph]

Yes, we decided this over in #1048 (comment). I will remove the lint rule. I just haven't gotten to it. Closing, please see that issue.