Closed pixelzoom closed 1 year ago
I added the type and used it in Challenge.ts. I'm not sure of an efficient way to find all the places it should be used.
This regex helps finds spots where primitives are being used as types to state objects.
type \w+StateObject = \{
I didn't see any string
usages. I did though see many number
and boolean`.
In my opinion. StringIO and BooleanIO do not need equivalents in type space, because they map literally to their primitive values. NumberStateObject is needed though to handle infinitiy. If we want to use a Type alias for string and boolean for consistency then should use it everywhere.
Ok. So actually @samreid and I went the opposite direction here in https://github.com/phetsims/tandem/issues/280. Why are we adding so much junk everywhere for primitive IOTypes? The answer was because NumberIO was serializing infinity, and the rest was mindless precedent following (at least by me). Things are much more cleaned up now. We don't need to serialize number/string/boolean, they are just that, and validators ensure that they are the right cloneable objects.
TLDR: There is no such thing as NumberStateObject, StringStateObject, or BooleanStateObject anymore. Just use the primitive values. In addition, you cannot call these IOType to/fromStateObjects, they are redundant, just pass through the value directly (guarded by a lint rule).
Reopening.
@zepumph said:
TLDR: There is no such thing as NumberStateObject, StringStateObject, or BooleanStateObject anymore. Just use the primitive values. In addition, you cannot call these IOType to/fromStateObjects, they are redundant, just pass through the value directly (guarded by a lint rule).
I still see NumberStateObject in NumberIO.ts. I still see 57 calls to NumberIO.toStateObject and 65 calls to NumberIO.fromStateObject.
I guess my working copy wasn't fully pulled. Or I pulled before the changes were fully pushed. I now see the changes to NumberIO.ts. Nice!
Reopening. Documentation for InfiniteNumberIO is identical to NumberIO, looks like a copy-paste error.
* IO Type for JS's built-in number type.
Let's pick this up in https://github.com/phetsims/tandem/issues/280 since that is where the commits are.
Most IOTypes have an associated state-object type. For example,
NumberIO
hasNumberStateObject
.StringIO
is different. It currently does not haveStringStateObject
. Existing implementations oftoStateObject
andfromStateObject
currently assume it’sstring
, and there’s no call toStringIO.toStateObject
orStringIO.fromeStateObject
. II recommend adding
StringStateObject
, because lack ofStringStateObject
creates (at least) 2 problems:(1) If StringIO needs to handle something other than
string
in the future, it will be painful to correct this omission. This is admittedly low probability, but not out of the question.(2) More importantly, it's confusing to treat StringIO differently, and it would be helpful/clearer if that the same pattern were used everywhere. Below is a general example that shows how (on several occassions) I've tried to treat StringIO like other IO types, then realized that there is no StringStateObject. I've then had to change my code as shown in the diff.
... only to find that there is no
StringStateObject
, and I need to usestring
.So... highly recommend to create
StringStateObject
, and use it where appropriate.