validateOptionsOnValidateValue in validator -> validateOptions option, …

…#207

js/Action.js

@@ -20,6 +20,8 @@ define( require => {
   const ValidatorDef = require( 'AXON/ValidatorDef' );
 
   // constants
+  const VALIDATE_OPTIONS_FALSE = { validateOptions: false };
+
   // Simulations have thousands of Emitters, so we re-use objects where possible.
   const EMPTY_ARRAY = [];
   assert && Object.freeze( EMPTY_ARRAY );
@@ -122,9 +124,6 @@ define( require => {
       for ( let i = 0; i < parameters.length; i++ ) {
         const parameter = parameters[ i ]; // metadata about a single parameter
 
-        assert && assert( parameter.validateOptionsOnValidateValue === undefined,
-          'Action sets its own validateOptionsOnValidateValue for each argument type'
-        );
         assert && assert( Object.getPrototypeOf( parameter ) === Object.prototype,
           'Extra prototype on parameter object is a code smell' );
 
@@ -147,11 +146,6 @@ define( require => {
           assert && assert( PARAMETER_KEYS.includes( key ), 'unrecognized parameter key: ' + key );
         }
 
-        // TODO: is this taking up too much memory? Does this create too much garbage? https://github.com/phetsims/axon/issues/257
-        parameters[ i ] = _.extend( {
-          validateOptionsOnValidateValue: false
-        }, parameter );
-
         // Changing after construction indicates a logic error.
         assert && Object.freeze( parameters[ i ] );
 
@@ -229,11 +223,11 @@ define( require => {
         );
         for ( let i = 0; i < this._parameters.length; i++ ) {
           const parameter = this._parameters[ i ];
-          validate( arguments[ i ], parameter );
+          validate( arguments[ i ], parameter, VALIDATE_OPTIONS_FALSE );
 
           // valueType overrides the phetioType validator so we don't use that one if there is a valueType
           if ( parameter.phetioType && !this._parameters.valueType ) {
-            validate( arguments[ i ], parameter.phetioType.validator );
+            validate( arguments[ i ], parameter.phetioType.validator, VALIDATE_OPTIONS_FALSE );
           }
         }
       }

js/Property.js

@@ -19,6 +19,9 @@ define( require => {
   const validate = require( 'AXON/validate' );
   const ValidatorDef = require( 'AXON/ValidatorDef' );
 
+  // constants
+  const VALIDATE_OPTIONS_FALSE = { validateOptions: false };
+
   // variables
   let globalId = 0; // autoincremented for unique IDs
 
@@ -46,10 +49,6 @@ define( require => {
         // cycles may pollute the data stream. See https://github.com/phetsims/axon/issues/179
         reentrant: false,
 
-        // By default, check the options once in the constructor, not on each subsequent value validation, to improve
-        // performance in requirejs mode
-        validateOptionsOnValidateValue: false
-
         // Property also supports validator options, see ValidatorDef.VALIDATOR_KEYS.
 
       }, options );
@@ -136,7 +135,7 @@ define( require => {
         }
         ValidatorDef.validateValidator( validator );
 
-        this.validate = value => validate( value, validator );
+        this.validate = value => validate( value, validator, VALIDATE_OPTIONS_FALSE );
 
         // validate the initial value
         this.validate( value );

js/ValidatorDef.js

@@ -77,19 +77,6 @@ define( require => {
     // {function(new: ObjectIO)} - A TypeIO used to specify the public typeing for PhET-iO. Each TypeIO must have a
     // `validator` key specified that can be used for validation. See ObjectIO for an example.
     'phetioType'
-
-    /**************************************
-     * Additionally, validation will always check the validator itself.  However, for types like Property and Emitter,
-     * re-checking the validator every time the Property value changes or the Emitter emits wastes time. Hence cases like
-     * those can opt-out by specifying:
-     *
-     * validateOptionsOnValidateValue: false
-     *
-     * Note: this should not be a key in VALIDATOR_KEYS because the keys are reserved for checking the value itself,
-     * see implementation and usage of containsValidatorKey.  Our "definition" of a validator is the makeup of the keys
-     * above, validateOptionsOnValidateValue is more of a meta-option that is not for checking the value itself, but
-     * whether to check the validator at the same time.
-     *********************/
   ];
 
   const ValidatorDef = {
@@ -238,10 +225,19 @@ define( require => {
      */
     isValueValid( value, validator, options ) {
 
-      options = options || ASSERTIONS_FALSE;
+      options = _.extend( {
+
+        // {boolean} - By default validation will always check the validity of the  validator itself.  However, for types like
+        // Property and Emitter re-checking the validator every time the Property value changes or the Emitter emits
+        // wastes cpu. Hence cases like those can opt-out
+        validateOptions: true,
+
+        // if true, throw an assertion "instead" of waiting to return a boolean
+        assertions: false
+      }, options );
 
       // Use the same policy for whether to throw assertions when checking the validator itself.
-      if ( validator.validateOptionsOnValidateValue !== false && !axon.ValidatorDef.isValidValidator( validator, options ) ) {
+      if ( options.validateOptions !== false && !axon.ValidatorDef.isValidValidator( validator, options ) ) {
         assert && options.assertions && assert( false, 'Invalid validator' );
         return false;
       }

js/ValidatorDefTests.js

@@ -165,7 +165,7 @@ define( require => {
     assert.ok( ValidatorDef.isValueValid( [ [], [], [], [] ], { arrayElementType: [ Array ] } ), 'array array' );
 
     window.assert && assert.throws( () => {
-      ValidatorDef.isValueValid( undefined, { arrayElementType: [ 'number', 'string' ], }, ASSERTIONS_TRUE );
+      ValidatorDef.isValueValid( undefined, { arrayElementType: [ 'number', 'string' ] }, ASSERTIONS_TRUE );
     }, 'undefined is not a valid arrayElementType' );
 
     window.assert && assert.throws( () => {

js/validate.js

@@ -17,15 +17,16 @@ define( require => {
    * If assertions are enabled, assert out if the value does not adhere to the validator. No-op without assertions.
    * @param {*} value
    * @param {ValidatorDef} validator
+   * @param {Object} [options] - see ValidatorDef.isValueValid()
    * @returns {*} - returns the input value for chaining
    * @public
    */
-  const validate = ( value, validator ) => {
+  const validate = ( value, validator, options ) => {
 
     if ( assert ) {
 
       // Throws an error if not valid
-      ValidatorDef.isValueValid( value, validator, { assertions: true } );
+      ValidatorDef.isValueValid( value, validator, _.extend( { assertions: true }, options ) );
     }
     return value;
   };