DerivedProperty/Multilink with TypeScript type parameters

[jonathanolson]

It looks like we should have property parameterized and type-safe DerivedProperty/Multilink.

For this, it's adding in an inferrable DerivedProperty type parameter (for the derivation parameters). Due to things needing IReadOnlyProperty (and instead of manually including the type parameter for DerivedProperty a lot), a lot of sim conversion has been done.

[jonathanolson]

Applied the commits above. @samreid can you review?

[pixelzoom]

Something seems wrong in DerivedProperty. I was under the impression that the types of callback params were inferrable from the types of the dependencies. But if I omit types for the callback params, I get TS errors.

Here's an example in geometric-optics.SecondPoint.js:

with:

readonly positionProperty: IReadOnlyProperty<Vector2>;

I don't understand the TS error. Why is it looking at a union of types, 'number | Vector2 | Representation' ?

[jonathanolson]

It's inferring... undefined for the types. And even more awkwardly, it's inferring [ undefined, undefined, undefined ] tuple for an array of 4 elements...

I'm at a loss, the inferred types don't seem to be functioning in this case, even though in most cases it should work.

[jonathanolson]

The specific error seems to be that it's not inferring the tuple correctly (and instead an array which the types get unioned).

This is what confuses me, since it should have 4 elements in the array.

[pixelzoom]

I don't have time to dive into DerivedProperty. So I guess I'll just continue to specify types for callback parameters.

[samreid]

I experimented with the idea in https://stackoverflow.com/a/45869621/1009071

Index: main/geometric-optics/js/common/model/SecondPoint.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/geometric-optics/js/common/model/SecondPoint.ts b/main/geometric-optics/js/common/model/SecondPoint.ts
--- a/main/geometric-optics/js/common/model/SecondPoint.ts	(revision 22a4875319087085abc9012bf04f991a01f756e7)
+++ b/main/geometric-optics/js/common/model/SecondPoint.ts	(date 1640196220830)
@@ -45,10 +45,11 @@
       range: VERTICAL_OFFSET_RANGE
     } );
 
-    this.positionProperty = new DerivedProperty(
-      [ sourceObjectPositionProperty, this.verticalOffsetProperty, this.lightSourcePositionProperty, representationProperty ],
-      ( sourceObjectPosition: Vector2, verticalOffset: number, lightSourcePosition: Vector2, representation: Representation ) =>
-        representation.isObject ? sourceObjectPosition.plusXY( 0, verticalOffset ) : lightSourcePosition
+    const a = new Property<number>();
+    const b = new Property<string>();
+    const positionProperty = new DerivedProperty(
+      [ a, b ],
+      ( sourceObjectPosition, verticalOffset ) => 'hello'
     );
 
     // @private
Index: main/axon/js/DerivedProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/axon/js/DerivedProperty.ts b/main/axon/js/DerivedProperty.ts
--- a/main/axon/js/DerivedProperty.ts	(revision b9d400a2d5cd811413e359fd43ade4c32d3b4cc3)
+++ b/main/axon/js/DerivedProperty.ts	(date 1640196343714)
@@ -27,12 +27,11 @@
 };
 
 // Maps tuples/arrays from T => IReadOnlyProperty<T>
-type MappedProperties<Parameters extends any[]> = {
-  [ K in keyof Parameters ]: IReadOnlyProperty<Parameters[K]>;
-};
+type MappedProperties<P1 = never, P2 = never, P3 = never> =
+  [ IReadOnlyProperty<P1>, IReadOnlyProperty<P2>, IReadOnlyProperty<P3> ];
 
 // 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 Derivation<T, P1, P2 = never, P3 = never> = ( p1: P1, p2: P2, p3: P3 ) => T;
 
 /**
  * Compute the derived value given a derivation and an array of dependencies
@@ -43,9 +42,9 @@
   return derivation( ...dependencies.map( property => property.get() ) );
 };
 
-class DerivedProperty<T, Parameters extends any[]> extends Property<T> implements IReadOnlyProperty<T> {
-  dependencies: MappedProperties<Parameters> | null;
-  derivation: Derivation<T, Parameters>;
+class DerivedProperty<T, P1 = never, P2 = never, P3 = never> extends Property<T> implements IReadOnlyProperty<T> {
+  dependencies: MappedProperties<[ P1, P2, P3 ]> | null;
+  derivation: Derivation<T, P1, P2, P3>;
   derivedPropertyListener: () => void;
   static DerivedPropertyIO: ( parameterType: any ) => any;
 
@@ -54,7 +53,7 @@
    * @param derivation - function that derives this Property's value, expects args in the same order as dependencies
    * @param [providedOptions] - see Property
    */
-  constructor( dependencies: MappedProperties<Parameters>, derivation: Derivation<T, Parameters>, providedOptions?: PropertyOptions<T> ) {
+  constructor( dependencies: [ IReadOnlyProperty<P1> | never, IReadOnlyProperty<P2> | never, IReadOnlyProperty<P3> | never ], derivation: Derivation<T, P1, P2, P3>, providedOptions?: PropertyOptions<T> ) {
 
     const options = merge( {
       tandem: Tandem.OPTIONAL,

[samreid]

@jbphet and I ran into this again today.

[samreid]

Fixed in #395, closing.