Better types for PropertyValues<T> (#2482)

* Better types for PropertyValues<T>

* Address review feedback

* Address feedback

.changeset/friendly-llamas-laugh.md

@@ -0,0 +1,6 @@
+---
+'@lit/reactive-element': patch
+'lit': patch
+---
+
+Update the definition of the PropertyValues type to give better types to `.get(k)`. `.get(k)` is now defined to return the correct type when using `PropertyValues<this>` and a parameter that's a key of the element class.

packages/reactive-element/src/reactive-element.ts

@@ -223,9 +223,18 @@ type AttributeMap = Map<string, PropertyKey>;
  * interface corresponding to the declared element properties.
  */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-export type PropertyValues<T = any> = keyof T extends PropertyKey
-  ? Map<keyof T, unknown>
-  : never;
+export interface PropertyValues<T = any> extends Map<keyof T, T[keyof T]> {
+  get<K extends keyof T>(k: K): T[K];
+  set<K extends keyof T>(key: K, value: T[K]): this;
+  forEach(
+    callbackfn: <K extends keyof T>(
+      value: T[K],
+      key: K,
+      map: Map<K, T[keyof T]>
+    ) => void,
+    thisArg?: unknown
+  ): void;
+}
 
 export const defaultConverter: ComplexAttributeConverter = {
   toAttribute(value: unknown, type?: unknown): unknown {

packages/reactive-element/src/test/reactive-element_test.ts

@@ -3074,5 +3074,60 @@ suite('ReactiveElement', () => {
       el2.setAttribute('foo', 'foo');
       assert.equal(el2.attrValue, 'custom');
     });
+
+    test('PropertyValues<this> type-checks', () => {
+      // This test only checks compile-type behavior. There are no runtime
+      // checks.
+      class E extends ReactiveElement {
+        declare foo: number;
+
+        override update(changedProperties: PropertyValues<this>) {
+          // @ts-expect-error 'bar' is not a keyof this
+          changedProperties.get('bar');
+          // @ts-expect-error 'bar' is not a keyof this
+          changedProperties.set('bar', 1);
+          // @ts-expect-error 'bar' is not a keyof this
+          changedProperties.has('bar');
+          // @ts-expect-error 'bar' is not a keyof this
+          changedProperties.delete('bar');
+          // @ts-expect-error number is not assignable to string
+          const w: string = changedProperties.get('foo');
+          // @ts-expect-error string is not assignable to number
+          changedProperties.set('foo', 'hi');
+
+          // This should type-check without a cast:
+          const x: number = changedProperties.get('foo');
+          changedProperties.set('foo', 2);
+
+          // This should type-check without a cast:
+          const propNames: Array<keyof this> = ['foo'];
+          const y = changedProperties.get(propNames[0]);
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          changedProperties.set(propNames[0], 1 as any);
+
+          changedProperties.forEach((v, k) => {
+            if (k === 'foo') {
+              // This assignment ideally _shouldn't_ fail. tsc should see that
+              // `k === 'foo'` implies `v is typeof this['foo']` (because v is
+              // `this[typeof k]`).
+              // @ts-expect-error tsc should be better
+              const z: number = v;
+              return z;
+            } else {
+              // @ts-expect-error Type 'this[K]' is not assignable to type
+              // 'number'.
+              const z: number = v;
+              return z;
+            }
+          });
+
+          // Suppress no-unused-vars warnings on x and y
+          return {x, y};
+        }
+      }
+      if (E) {
+        // Suppress no-unused-vars warning on E
+      }
+    });
   });
 });