samreid
commented
3 years ago The Proxy-based solution is working well, I think it has compatibility with Array API. I also added adapters to give compatibility with ObservableArray API to aid in migration. I took it for a test drive in Natural Selection, and it was straightforward to convert ObservableArray => createArrayProxy. Even though this doesn't use subtyping, there is a way to simulate BunnyArray (not as nice as subclassing, but it seems workable).
```diff
Index: js/common/model/PopulationModel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/common/model/PopulationModel.js (revision eef23203cb7af9cf7931b75a1e16cf915abd545b)
+++ js/common/model/PopulationModel.js (date 1600558272085)
@@ -7,9 +7,9 @@
*/
import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
+import createArrayProxy from '../../../../axon/js/createArrayProxy.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import NumberProperty from '../../../../axon/js/NumberProperty.js';
-import ObservableArray from '../../../../axon/js/ObservableArray.js';
import Property from '../../../../axon/js/Property.js';
import PropertyIO from '../../../../axon/js/PropertyIO.js';
import Range from '../../../../dot/js/Range.js';
@@ -87,39 +87,39 @@
// @public {ObservableArray.} - data points, for total population and the population of each allele.
// Vector2.x = generation, Vector2.y = population
- this.totalPoints = new ObservableArray( {
+ this.totalPoints = createArrayProxy( {
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
tandem: dataPointsTandem.createTandem( 'totalPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
phetioDocumentation: 'Population data points for all bunnies'
} );
- this.whiteFurPoints = new ObservableArray( {
+ this.whiteFurPoints = createArrayProxy( {
tandem: dataPointsTandem.createTandem( 'whiteFurPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
phetioDocumentation: 'Population data points for bunnies with white fur'
} );
- this.brownFurPoints = new ObservableArray( {
+ this.brownFurPoints = createArrayProxy( {
tandem: dataPointsTandem.createTandem( 'brownFurPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
- phetioDocumentation: 'Population data points for bunnies with brown fur'
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
+ phetioDocumentation: 'Population data points for bunnies with brown fur',
} );
- this.straightEarsPoints = new ObservableArray( {
+ this.straightEarsPoints = createArrayProxy( {
tandem: dataPointsTandem.createTandem( 'straightEarsPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
phetioDocumentation: 'Population data points for bunnies with straight ears'
} );
- this.floppyEarsPoints = new ObservableArray( {
+ this.floppyEarsPoints = createArrayProxy( {
tandem: dataPointsTandem.createTandem( 'floppyEarsPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
phetioDocumentation: 'Population data points for bunnies with floppy ears'
} );
- this.shortTeethPoints = new ObservableArray( {
+ this.shortTeethPoints = createArrayProxy( {
tandem: dataPointsTandem.createTandem( 'shortTeethPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
phetioDocumentation: 'Population data points for bunnies with short teeth'
} );
- this.longTeethPoints = new ObservableArray( {
+ this.longTeethPoints = createArrayProxy( {
tandem: dataPointsTandem.createTandem( 'longTeethPoints' ),
- phetioType: ObservableArray.ObservableArrayIO( Vector2IO ),
+ phetioType: createArrayProxy.ArrayProxyIO( Vector2IO ),
phetioDocumentation: 'Population data points for bunnies with long teeth'
} );
@@ -281,7 +281,7 @@
* @param {number} count
*/
function recordCount( observableArray, timeInGenerations, count ) {
- assert && assert( observableArray instanceof ObservableArray, 'invalid observableArray' );
+ assert && assert( Array.isArray( observableArray ), 'invalid observableArray' );
assert && assert( NaturalSelectionUtils.isNonNegative( timeInGenerations ), 'invalid generation' );
assert && assert( NaturalSelectionUtils.isNonNegativeInteger( count ), 'invalid count' );
Index: js/common/model/BunnyArray.js
===================================================================
--- js/common/model/BunnyArray.js (revision eef23203cb7af9cf7931b75a1e16cf915abd545b)
+++ js/common/model/createBunnyArray.js (date 1600558327840)
@@ -6,7 +6,7 @@
* @author Chris Malley (PixelZoom, Inc.)
*/
-import ObservableArray from '../../../../axon/js/ObservableArray.js';
+import createArrayProxy from '../../../../axon/js/createArrayProxy.js';
import Property from '../../../../axon/js/Property.js';
import PropertyIO from '../../../../axon/js/PropertyIO.js';
import merge from '../../../../phet-core/js/merge.js';
@@ -16,52 +16,42 @@
import Bunny from './Bunny.js';
import BunnyCounts from './BunnyCounts.js';
-class BunnyArray extends ObservableArray {
-
- /**
- * @param {Object} [options]
- */
- constructor( options ) {
-
- options = merge( {
+const createBunnyArray = options => {
+ options = merge( {
- // phet-io
- tandem: Tandem.REQUIRED,
- phetioType: ObservableArray.ObservableArrayIO( ReferenceIO( Bunny.BunnyIO ) ),
- phetioState: false
- }, options );
-
- super( options );
+ phetioType: createArrayProxy.ArrayProxyIO( ReferenceIO( Bunny.BunnyIO ) ),
+
+ // phet-io
+ tandem: Tandem.REQUIRED,
+ phetioState: false
+ }, options );
+ const bunnyArray = createArrayProxy( options );
- // @public (read-only) {Property.}
- this.countsProperty = new Property( BunnyCounts.withZero(), {
- tandem: options.tandem.createTandem( 'countsProperty' ),
- phetioType: PropertyIO( BunnyCounts.BunnyCountsIO ),
- phetioState: false // because counts will be restored as Bunny instances are restored to BunnyGroup
- } );
+ // @public (read-only) {Property.}
+ bunnyArray.countsProperty = new Property( BunnyCounts.withZero(), {
+ tandem: options.tandem.createTandem( 'countsProperty' ),
+ phetioType: PropertyIO( BunnyCounts.BunnyCountsIO ),
+ phetioState: false // because counts will be restored as Bunny instances are restored to BunnyGroup
+ } );
- // Update counts when a bunny is added. removeItemAddedListener is not necessary.
- this.addItemAddedListener( bunny => {
- this.countsProperty.value = this.countsProperty.value.plus( bunny );
- assert && assert( this.countsProperty.value.totalCount === this.length, 'counts out of sync' );
- } );
+ // Update counts when a bunny is added. removeItemAddedListener is not necessary.
+ bunnyArray.addItemAddedListener( bunny => {
+ bunnyArray.countsProperty.value = bunnyArray.countsProperty.value.plus( bunny );
+ assert && assert( bunnyArray.countsProperty.value.totalCount === bunnyArray.length, 'counts out of sync' );
+ } );
- // Update counts when a bunny is removed. removeItemAddedListener is not necessary.
- this.addItemRemovedListener( bunny => {
- this.countsProperty.value = this.countsProperty.value.minus( bunny );
- assert && assert( this.countsProperty.value.totalCount === this.length, 'counts out of sync' );
- } );
- }
+ // Update counts when a bunny is removed. removeItemAddedListener is not necessary.
+ bunnyArray.addItemRemovedListener( bunny => {
+ bunnyArray.countsProperty.value = bunnyArray.countsProperty.value.minus( bunny );
+ assert && assert( bunnyArray.countsProperty.value.totalCount === bunnyArray.length, 'counts out of sync' );
+ } );
- /**
- * @public
- * @override
- */
- dispose() {
- assert && assert( false, 'dispose is not supported, exists for the lifetime of the sim' );
- super.dispose();
- }
-}
+ bunnyArray.dispose = () => {
+ assert && assert( false, 'dispose not supported, exists for the life of the sim' );
+ };
-naturalSelection.register( 'BunnyArray', BunnyArray );
-export default BunnyArray;
\ No newline at end of file
+ return bunnyArray;
+};
+
+naturalSelection.register( 'createBunnyArray', createBunnyArray );
+export default createBunnyArray;
\ No newline at end of file
Index: js/common/model/BunnyCollection.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/common/model/BunnyCollection.js (revision eef23203cb7af9cf7931b75a1e16cf915abd545b)
+++ js/common/model/BunnyCollection.js (date 1600555796008)
@@ -20,7 +20,7 @@
import NaturalSelectionQueryParameters from '../NaturalSelectionQueryParameters.js';
import NaturalSelectionUtils from '../NaturalSelectionUtils.js';
import Bunny from './Bunny.js';
-import BunnyArray from './BunnyArray.js';
+import createBunnyArray from './createBunnyArray.js';
import BunnyGroup from './BunnyGroup.js';
import EnvironmentModelViewTransform from './EnvironmentModelViewTransform.js';
import GenePool from './GenePool.js';
@@ -64,12 +64,12 @@
}, options );
// @public (read-only) the live bunnies in bunnyGroup
- this.liveBunnies = new BunnyArray( {
+ this.liveBunnies = createBunnyArray( {
tandem: options.tandem.createTandem( 'liveBunnies' )
} );
// @private the dead bunnies in bunnyGroup
- this.deadBunnies = new BunnyArray( {
+ this.deadBunnies = createBunnyArray( {
tandem: options.tandem.createTandem( 'deadBunnies' )
} );
@@ -77,7 +77,7 @@
// soon as possible. Mutants are added to this array when born, and removed as soon as they have mated with
// another bunny that has the same mutant allele. See also the 'Recessive Mutants' section of model.md at
// https://github.com/phetsims/natural-selection/blob/master/doc/model.md#recessive-mutants.
- this.recessiveMutants = new BunnyArray( {
+ this.recessiveMutants = createBunnyArray( {
tandem: options.tandem.createTandem( 'recessiveMutants' ),
phetioDocumentation: 'for internal PhET use only'
} );
```
jonathanolson
Denz1994
zepumph
pixelzoom
From https://github.com/phetsims/axon/issues/311, we would like to create a Proxy-based Array implementation that has full API compatibility, which will make it easier to migrate from plain arrays to arrays that can be observed. We discussed that this solution will likely have these disadvantages:
BunnyArray extends AxonArrayProxyWe agreed it will be OK to not be able to subclass this solution (difficulty in extending Proxy discussed in https://github.com/phetsims/axon/issues/311#issuecomment-682882281), but that we will need to characterize performance to see if it is acceptable. Our main concern is that if one Array Proxy is passed to performance-critical code, it could deoptimize all of its calls.