DynamicProperty derive strings don't work with parametric types

[samreid]

@matthew-blackman and I converted a type to be parametric and found that DynamicProperty derive no longer worked. We would like to investigate that.

In PDLModel.ts, we had to change:

    this.launcherHeightProperty = new DynamicProperty<number, number, Field>( this.fieldProperty, {
      bidirectional: true,
      derive: 'launcherHeightProperty'
    } );

to

    this.launcherHeightProperty = new DynamicProperty<number, number, T>( this.fieldProperty, {
      bidirectional: true,
      derive: t => t.launcherHeightProperty // string no longer works here even though T extends Field
    } );

[samreid]

This works, but still feels like a workaround.

Subject: [PATCH] Improve timing between projectiles, see https://github.com/phetsims/projectile-data-lab/issues/7
---
Index: js/common/model/PDLModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/PDLModel.ts b/js/common/model/PDLModel.ts
--- a/js/common/model/PDLModel.ts	(revision b95e6ea491ed063ac4fd1ee6c1f4d0280911f9eb)
+++ b/js/common/model/PDLModel.ts	(date 1701215400635)
@@ -57,7 +57,7 @@
   // TODO: Don't use number, see https://github.com/phetsims/projectile-data-lab/issues/7
   public readonly launcherTypeProperty: DynamicProperty<number, number, T>;
 
-  public readonly launcherAngleProperty: DynamicProperty<number, number, T>;
+  public readonly launcherAngleProperty: DynamicProperty<number, number, Field>;
   public readonly launcherHeightProperty: DynamicProperty<number, number, T>;
 
   public readonly isPathsVisibleProperty: BooleanProperty;
@@ -121,7 +121,7 @@
       derive: t => t.launcherTypeProperty
     } );
 
-    this.launcherAngleProperty = new DynamicProperty<number, number, T>( this.fieldProperty, {
+    this.launcherAngleProperty = new DynamicProperty<number, number, Field>( this.fieldProperty, {
       bidirectional: true,
       derive: t => t.launcherAngleProperty
     } );

[samreid]

I worked on this for about 20 minutes yesterday, and have no leads. We may want to request help from @zepumph or @jonathanolson. We have a great workaround, but wanted to use this as an opportunity to improve DynamicProperty (if there is a type checking issue). @jonathanolson or @zepumph want to take a look?

[zepumph]

Taking a look now.

[samreid]

@zepumph and I investigated with this patch:

Subject: [PATCH] Remove stale code comment, see https://github.com/phetsims/chipper/issues/1245
---
Index: axon/js/DynamicProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/axon/js/DynamicProperty.ts b/axon/js/DynamicProperty.ts
--- a/axon/js/DynamicProperty.ts	(revision 799a2a1caebe47ca63f9e753174ecb9b6bd6ca35)
+++ b/axon/js/DynamicProperty.ts	(date 1701281164367)
@@ -120,7 +120,12 @@
   // If it's a string, it will grab that named property out (e.g. it's like passing u => u[ derive ])
   // NOTE: This accepts TReadOnlyProperty, but if you have bidirectional:true it must be a full TProperty.
   // This is not currently type checked.
-  derive?: ( ( outerValue: OuterValueType ) => TReadOnlyProperty<InnerValueType> ) | KeysMatching<OuterValueType, TReadOnlyProperty<InnerValueType>>;
+  derive?:
+
+    // ( ( outerValue: OuterValueType ) => TReadOnlyProperty<InnerValueType> ) |
+
+
+    KeysMatching<OuterValueType, TReadOnlyProperty<InnerValueType>>;
 
   // Maps our input Property value to/from this Property's value. See top-level documentation for usage.
   // If it's a string, it will grab that named property out (e.g. it's like passing u => u[ derive ])
Index: phet-core/js/types/KeysMatching.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/phet-core/js/types/KeysMatching.ts b/phet-core/js/types/KeysMatching.ts
--- a/phet-core/js/types/KeysMatching.ts	(revision d24b34031b9b94609613c11a8b5d265dc441c0ea)
+++ b/phet-core/js/types/KeysMatching.ts	(date 1701281338583)
@@ -9,5 +9,8 @@
  * @author Jonathan Olson <jonathan.olson@colorado.edu>
  */
 
-type KeysMatching<T, V> = { [K in keyof T]-?: T[K] extends V ? K : never }[keyof T];
+// type KeysMatching<T, V> = { [K in keyof T]-?: T[K] extends V ? K : never }[keyof T];
+
+type KeysMatching<T, V> = keyof { [P in keyof T as T[P] extends V ? P : never]: P };
+
 export default KeysMatching;
Index: projectile-data-lab/js/common/model/PDLModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/projectile-data-lab/js/common/model/PDLModel.ts b/projectile-data-lab/js/common/model/PDLModel.ts
--- a/projectile-data-lab/js/common/model/PDLModel.ts	(revision 5e3908e27f5ccbca471dd3416e3b7263c4d26a37)
+++ b/projectile-data-lab/js/common/model/PDLModel.ts	(date 1701281164376)
@@ -23,6 +23,7 @@
 import BooleanIO from '../../../../tandem/js/types/BooleanIO.js';
 import TProperty from '../../../../axon/js/TProperty.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
+import KeysMatching from '../../../../phet-core/js/types/KeysMatching.js';
 
 type SelfOptions<T extends Field> = {
   timeSpeedValues: TimeSpeed[];
@@ -125,9 +126,21 @@
       derive: t => t.launcherAngleProperty
     } );
 
+    const x:T = 'hello';
+    x.launcherHeightProperty;
+    type m = KeysMatching<T, TReadOnlyProperty<number>>;
+    type z = KeysMatching<Field, TReadOnlyProperty<number>>;
+
+    const a1: z = 'launcherHeightProperty';
+    const a2: m = 'launcherHeightProperty';
+
+
+    console.log(a1,a2);
+
+
     this.launcherHeightProperty = new DynamicProperty<number, number, T>( this.fieldProperty, {
       bidirectional: true,
-      derive: t => t.launcherHeightProperty
+      derive: 'launcherHeightProperty'
     } );
 
     this.isPathsVisibleProperty = new BooleanProperty( providedOptions.isPathsVisible, {

Note this part:

    type m = KeysMatching<T, TReadOnlyProperty<number>>;
    type z = KeysMatching<Field, TReadOnlyProperty<number>>;

    const a1: z = 'launcherHeightProperty'; //FINE
    const a2: m = 'launcherHeightProperty';// ERROR

The problem is known in TypeScript and documented in https://stackoverflow.com/questions/54520676/in-typescript-how-to-get-the-keys-of-an-object-type-whose-values-are-of-a-given and microsoft/TypeScript#48992 and microsoft/TypeScript#48989, so it sounds like we will need to use a workaround until this is solved in TypeScript (if it ever is).

We would also document this issue in DynamicProperty at the options site.

[zepumph]

Anything else you'd like from me here?

[samreid]

That's all, thanks @zepumph! I added some documentation in DynamicProperty, closing.