phetsims / phet-core

Core utilities used by all PhET simulations.
MIT License
8 stars 6 forks source link

Rename Defaults to OptionizeDefaults. #107

Closed pixelzoom closed 2 years ago

pixelzoom commented 2 years ago

In optionize.ts:

export type { Defaults };

This name is too general. Let's rename to OptionizeDefaults.

zepumph commented 2 years ago

How about DefaultOptions?

pixelzoom commented 2 years ago

DefaultOptions is still too general imo. Defaults for what? OptionizeDefaults is only 3 chars longer..

The documentation for Defaults says that's it's the defaults for optionize:

// This is the type for the `defaults` argument to optionize
type Defaults<SelfOptions = ...

... so why not name it as such?

pixelzoom commented 2 years ago

Here's a typical example from geometric-optics (FocalLengthControl.ts). What name would look best in this context, while still being sufficiently descriptive?

// Assemble the defaults for NumberControl, because optionize doesn't currently support defaults in multiple objects.
const numberControlDefaults: Defaults<{}, NumberControlOptions> = merge( {}, ..., {
 ...
} );

const options = optionize<FocalLengthControlOptions, {}, NumberControlOptions>( {}, numberControlDefaults, providedOptions );
zepumph commented 2 years ago

I like OptionizeDefaults, thanks for talking it through with me. Please review.

pixelzoom commented 2 years ago

👍🏻 closing