diff --git a/js/Action.js b/js/Action.js index 3bdd188a..0d96f349 100644 --- a/js/Action.js +++ b/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 ); } } } diff --git a/js/Property.js b/js/Property.js index c0ce2e5b..8a4b1ad1 100644 --- a/js/Property.js +++ b/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 ); diff --git a/js/ValidatorDef.js b/js/ValidatorDef.js index 162d0859..8ace7d10 100644 --- a/js/ValidatorDef.js +++ b/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; } diff --git a/js/ValidatorDefTests.js b/js/ValidatorDefTests.js index 9dd52ca1..0bc28300 100644 --- a/js/ValidatorDefTests.js +++ b/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( () => { diff --git a/js/validate.js b/js/validate.js index 9cab86b4..99b953c2 100644 --- a/js/validate.js +++ b/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; };