Skip to content

Commit

Permalink
Omit phetioDynamicElement option, make phetioDynamicElementName optio…
Browse files Browse the repository at this point in the history
…nal and improve type for PhetioGroupOptions, see #254
  • Loading branch information
samreid committed May 9, 2022
1 parent e97be0c commit 79234c9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
10 changes: 7 additions & 3 deletions js/PhetioDynamicElementContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ type SelfOptions = {
containerSuffix?: string;

// tandem name for elements in the container is the container's tandem name without containerSuffix
phetioDynamicElementName: string;
phetioDynamicElementName?: string;
}

type PhetioDynamicElementContainerOptions = SelfOptions & PhetioObjectOptions & PickRequired<PhetioObjectOptions, 'phetioType'>;
export type PhetioDynamicElementContainerOptions = SelfOptions & Omit<PhetioObjectOptions, 'phetioDynamicElement'> & PickRequired<PhetioObjectOptions, 'phetioType'>;


type ClearOptions = {
Expand Down Expand Up @@ -92,7 +92,11 @@ abstract class PhetioDynamicElementContainer<T extends PhetioObject, P extends a
// Many PhET-iO instrumented types live in common code used by multiple sims, and may only be instrumented in a subset of their usages.
tandem: Tandem.OPTIONAL,
supportsDynamicState: true,
containerSuffix: DEFAULT_CONTAINER_SUFFIX
containerSuffix: DEFAULT_CONTAINER_SUFFIX,

// TODO: https://github.com/phetsims/tandem/issues/254
// @ts-ignore - this is filled in below
phetioDynamicElementName: undefined
}, providedOptions );

assert && assert( typeof createElement === 'function', 'createElement should be a function' );
Expand Down
14 changes: 9 additions & 5 deletions js/PhetioGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
import NumberProperty from '../../axon/js/NumberProperty.js';
import arrayRemove from '../../phet-core/js/arrayRemove.js';
import merge from '../../phet-core/js/merge.js';
import PhetioDynamicElementContainer from './PhetioDynamicElementContainer.js';
import optionize from '../../phet-core/js/optionize.js';
import PhetioDynamicElementContainer, { PhetioDynamicElementContainerOptions } from './PhetioDynamicElementContainer.js';
import PhetioObject from './PhetioObject.js';
import Tandem from './Tandem.js';
import tandemNamespace from './tandemNamespace.js';
Expand All @@ -29,6 +30,9 @@ const DEFAULT_CONTAINER_SUFFIX = 'Group';
// Type of a derivation function, that returns T and takes the typed parameters (as a tuple type)
// type Derivation<T, Parameters extends any[]> = ( ...params: Parameters ) => T;

type SelfOptions = {};
export type PhetioGroupOptions = SelfOptions & PhetioDynamicElementContainerOptions;

// A extends ( ...args: any[] ) => any, B extends Parameters<A>
class PhetioGroup<T extends PhetioObject, P extends any[] = []> extends PhetioDynamicElementContainer<T, P> {
private readonly _array: T[];
Expand All @@ -43,17 +47,17 @@ class PhetioGroup<T extends PhetioObject, P extends any[] = []> extends PhetioDy
* @param {Array.<*>|function():Array.<*>} defaultArguments - arguments passed to createElement when creating the archetype.
* Note: if `createElement` supports options, but don't need options for this
* defaults array, you should pass an empty object here anyways.
* @param {Object} [options] - describe the Group itself
* @param {Object} [providedOptions] - describe the Group itself
*/
constructor( createElement: ( t: Tandem, ...p: P ) => T, defaultArguments: P | ( () => P ), options?: any ) {
constructor( createElement: ( t: Tandem, ...p: P ) => T, defaultArguments: P | ( () => P ), providedOptions?: PhetioGroupOptions ) {

options = merge( {
const options = optionize<PhetioGroupOptions, SelfOptions, PhetioDynamicElementContainerOptions>()( {
tandem: Tandem.OPTIONAL,

// {string} The group's tandem name must have this suffix, and the base tandem name for elements of
// the group will consist of the group's tandem name with this suffix stripped off.
containerSuffix: DEFAULT_CONTAINER_SUFFIX
}, options );
}, providedOptions );

super( createElement, defaultArguments, options );

Expand Down

0 comments on commit 79234c9

Please sign in to comment.