[labs/observers] Improve controllers value type from unknown to gener…

…ic (#3294) Fix value property of type `unknown` on exported controllers. The type of `value` is now generic and can be inferred from the return type of your passed in `callback`. The default callback `() => true` was removed, and is now undefined by default.
lit · Sep 30, 2022 · 96c05f2 · 96c05f2
1 parent 31bed8d
commit 96c05f2
Show file tree

Hide file tree

Showing 9 changed files with 240 additions and 60 deletions.
diff --git a/.changeset/seven-fireants-wonder.md b/.changeset/seven-fireants-wonder.md
@@ -0,0 +1,8 @@
+---
+'@lit-labs/observers': minor
+---
+
+Fix value property of type `unknown` on exported controllers. The type of
+`value` is now generic and can be inferred from the return type of your passed
+in `callback`. The default callback `() => true` was removed, and is now
+undefined by default.
diff --git a/packages/labs/observers/src/intersection_controller.ts b/packages/labs/observers/src/intersection_controller.ts
@@ -11,14 +11,14 @@ import {
 /**
  * The callback function for a IntersectionController.
  */
-export type IntersectionValueCallback = (
+export type IntersectionValueCallback<T = unknown> = (
   ...args: Parameters<IntersectionObserverCallback>
-) => unknown;
+) => T;
 
 /**
  * The config options for a IntersectionController.
  */
-export interface IntersectionControllerConfig {
+export interface IntersectionControllerConfig<T = unknown> {
   /**
    * Configuration object for the IntersectionObserver.
    */
@@ -35,7 +35,7 @@ export interface IntersectionControllerConfig {
    * The callback used to process detected changes into a value stored
    * in the controller's `value` property.
    */
-  callback?: IntersectionValueCallback;
+  callback?: IntersectionValueCallback<T>;
   /**
    * An IntersectionObserver reports the initial intersection state
    * when observe is called. Setting this flag to true skips processing this
@@ -60,7 +60,7 @@ export interface IntersectionControllerConfig {
  * used to process the result into a value which is stored on the controller.
  * The controller's `value` is usable during the host's update cycle.
  */
-export class IntersectionController implements ReactiveController {
+export class IntersectionController<T = unknown> implements ReactiveController {
   private _host: ReactiveControllerHost;
   private _target: Element | null;
   private _observer!: IntersectionObserver;
@@ -77,22 +77,22 @@ export class IntersectionController implements ReactiveController {
    * The result of processing the observer's changes via the `callback`
    * function.
    */
-  value?: unknown;
+  value?: T;
   /**
    * Function that returns a value processed from the observer's changes.
    * The result is stored in the `value` property.
    */
-  callback: IntersectionValueCallback = () => true;
+  callback?: IntersectionValueCallback<T>;
   constructor(
     host: ReactiveControllerHost,
-    {target, config, callback, skipInitial}: IntersectionControllerConfig
+    {target, config, callback, skipInitial}: IntersectionControllerConfig<T>
   ) {
     this._host = host;
     // Target defaults to `host` unless explicitly `null`.
     this._target =
       target === null ? target : target ?? (this._host as unknown as Element);
     this._skipInitial = skipInitial ?? this._skipInitial;
-    this.callback = callback ?? this.callback;
+    this.callback = callback;
     // Check browser support.
     if (!window.IntersectionObserver) {
       console.warn(
@@ -120,7 +120,7 @@ export class IntersectionController implements ReactiveController {
    * function to produce a result stored in the `value` property.
    */
   protected handleChanges(entries: IntersectionObserverEntry[]) {
-    this.value = this.callback(entries, this._observer);
+    this.value = this.callback?.(entries, this._observer);
   }
 
   hostConnected() {

diff --git a/packages/labs/observers/src/mutation_controller.ts b/packages/labs/observers/src/mutation_controller.ts
@@ -11,14 +11,14 @@ import {
 /**
  * The callback function for a MutationController.
  */
-export type MutationValueCallback = (
+export type MutationValueCallback<T = unknown> = (
   ...args: Parameters<MutationCallback>
-) => unknown;
+) => T;
 
 /**
  * The config options for a MutationController.
  */
-export interface MutationControllerConfig {
+export interface MutationControllerConfig<T = unknown> {
   /**
    * Configuration object for the MutationObserver.
    */
@@ -35,7 +35,7 @@ export interface MutationControllerConfig {
    * The callback used to process detected changes into a value stored
    * in the controller's `value` property.
    */
-  callback?: MutationValueCallback;
+  callback?: MutationValueCallback<T>;
   /**
    * By default the `callback` is called without changes when a target is
    * observed. This is done to help manage initial state, but this
@@ -59,7 +59,7 @@ export interface MutationControllerConfig {
  * used to process the result into a value which is stored on the controller.
  * The controller's `value` is usable during the host's update cycle.
  */
-export class MutationController implements ReactiveController {
+export class MutationController<T = unknown> implements ReactiveController {
   private _host: ReactiveControllerHost;
   private _target: Element | null;
   private _config: MutationObserverInit;
@@ -76,23 +76,23 @@ export class MutationController implements ReactiveController {
    * The result of processing the observer's changes via the `callback`
    * function.
    */
-  value?: unknown;
+  value?: T;
   /**
    * Function that returns a value processed from the observer's changes.
    * The result is stored in the `value` property.
    */
-  callback: MutationValueCallback = () => true;
+  callback?: MutationValueCallback<T>;
   constructor(
     host: ReactiveControllerHost,
-    {target, config, callback, skipInitial}: MutationControllerConfig
+    {target, config, callback, skipInitial}: MutationControllerConfig<T>
   ) {
     this._host = host;
     // Target defaults to `host` unless explicitly `null`.
     this._target =
       target === null ? target : target ?? (this._host as unknown as Element);
     this._config = config;
     this._skipInitial = skipInitial ?? this._skipInitial;
-    this.callback = callback ?? this.callback;
+    this.callback = callback;
     // Check browser support.
     if (!window.MutationObserver) {
       console.warn(
@@ -112,7 +112,7 @@ export class MutationController implements ReactiveController {
    * function to produce a result stored in the `value` property.
    */
   protected handleChanges(records: MutationRecord[]) {
-    this.value = this.callback(records, this._observer);
+    this.value = this.callback?.(records, this._observer);
   }
 
   hostConnected() {

diff --git a/packages/labs/observers/src/performance_controller.ts b/packages/labs/observers/src/performance_controller.ts
@@ -11,16 +11,16 @@ import {
 /**
  * The callback function for a PerformanceController.
  */
-export type PerformanceValueCallback = (
+export type PerformanceValueCallback<T = unknown> = (
   entries: PerformanceEntryList,
   observer: PerformanceObserver,
   entryList?: PerformanceObserverEntryList
-) => unknown;
+) => T;
 
 /**
  * The config options for a PerformanceController.
  */
-export interface PerformanceControllerConfig {
+export interface PerformanceControllerConfig<T = unknown> {
   /**
    * Configuration object for the PerformanceObserver.
    */
@@ -29,7 +29,7 @@ export interface PerformanceControllerConfig {
    * The callback used to process detected changes into a value stored
    * in the controller's `value` property.
    */
-  callback?: PerformanceValueCallback;
+  callback?: PerformanceValueCallback<T>;
   /**
    * By default the `callback` is called without changes when a target is
    * observed. This is done to help manage initial state, but this
@@ -50,7 +50,7 @@ export interface PerformanceControllerConfig {
  * used to process the result into a value which is stored on the controller.
  * The controller's `value` is usable during the host's update cycle.
  */
-export class PerformanceController implements ReactiveController {
+export class PerformanceController<T = unknown> implements ReactiveController {
   private _host: ReactiveControllerHost;
   private _config: PerformanceObserverInit;
   private _observer!: PerformanceObserver;
@@ -66,20 +66,20 @@ export class PerformanceController implements ReactiveController {
    * The result of processing the observer's changes via the `callback`
    * function.
    */
-  value?: unknown;
+  value?: T;
   /**
    * Function that returns a value processed from the observer's changes.
    * The result is stored in the `value` property.
    */
-  callback: PerformanceValueCallback = () => true;
+  callback?: PerformanceValueCallback<T>;
   constructor(
     host: ReactiveControllerHost,
-    {config, callback, skipInitial}: PerformanceControllerConfig
+    {config, callback, skipInitial}: PerformanceControllerConfig<T>
   ) {
     this._host = host;
     this._config = config;
     this._skipInitial = skipInitial ?? this._skipInitial;
-    this.callback = callback ?? this.callback;
+    this.callback = callback;
     // Check browser support.
     if (!window.PerformanceObserver) {
       console.warn(
@@ -104,7 +104,7 @@ export class PerformanceController implements ReactiveController {
     entries: PerformanceEntryList,
     entryList?: PerformanceObserverEntryList
   ) {
-    this.value = this.callback(entries, this._observer, entryList);
+    this.value = this.callback?.(entries, this._observer, entryList);
   }
 
   hostConnected() {

diff --git a/packages/labs/observers/src/resize_controller.ts b/packages/labs/observers/src/resize_controller.ts
@@ -11,14 +11,14 @@ import {
 /**
  * The callback function for a ResizeController.
  */
-export type ResizeValueCallback = (
+export type ResizeValueCallback<T = unknown> = (
   ...args: Parameters<ResizeObserverCallback>
-) => unknown;
+) => T;
 
 /**
  * The config options for a ResizeController.
  */
-export interface ResizeControllerConfig {
+export interface ResizeControllerConfig<T = unknown> {
   /**
    * Configuration object for the ResizeController.
    */
@@ -35,7 +35,7 @@ export interface ResizeControllerConfig {
    * The callback used to process detected changes into a value stored
    * in the controller's `value` property.
    */
-  callback?: ResizeValueCallback;
+  callback?: ResizeValueCallback<T>;
   /**
    * By default the `callback` is called without changes when a target is
    * observed. This is done to help manage initial state, but this
@@ -58,7 +58,7 @@ export interface ResizeControllerConfig {
  * used to process the result into a value which is stored on the controller.
  * The controller's `value` is usable during the host's update cycle.
  */
-export class ResizeController implements ReactiveController {
+export class ResizeController<T = unknown> implements ReactiveController {
   private _host: ReactiveControllerHost;
   private _target: Element | null;
   private _config?: ResizeObserverOptions;
@@ -75,23 +75,23 @@ export class ResizeController implements ReactiveController {
    * The result of processing the observer's changes via the `callback`
    * function.
    */
-  value?: unknown;
+  value?: T;
   /**
    * Function that returns a value processed from the observer's changes.
    * The result is stored in the `value` property.
    */
-  callback: ResizeValueCallback = () => true;
+  callback?: ResizeValueCallback<T>;
   constructor(
     host: ReactiveControllerHost,
-    {target, config, callback, skipInitial}: ResizeControllerConfig
+    {target, config, callback, skipInitial}: ResizeControllerConfig<T>
   ) {
     this._host = host;
     // Target defaults to `host` unless explicitly `null`.
     this._target =
       target === null ? target : target ?? (this._host as unknown as Element);
     this._config = config;
     this._skipInitial = skipInitial ?? this._skipInitial;
-    this.callback = callback ?? this.callback;
+    this.callback = callback;
     // Check browser support.
     if (!window.ResizeObserver) {
       console.warn(
@@ -111,7 +111,7 @@ export class ResizeController implements ReactiveController {
    * function to produce a result stored in the `value` property.
    */
   protected handleChanges(entries: ResizeObserverEntry[]) {
-    this.value = this.callback(entries, this._observer);
+    this.value = this.callback?.(entries, this._observer);
   }
 
   hostConnected() {

diff --git a/packages/labs/observers/src/test/intersection_controller_test.ts b/packages/labs/observers/src/test/intersection_controller_test.ts
@@ -12,6 +12,7 @@ import {
 import {
   IntersectionController,
   IntersectionControllerConfig,
+  IntersectionValueCallback,
 } from '@lit-labs/observers/intersection_controller.js';
 import {generateElementName, nextFrame} from './test-helpers.js';
 import {assert} from '@esm-bundle/chai';
@@ -55,7 +56,10 @@ const canTest = () => {
       constructor() {
         super();
         const config = getControllerConfig(this);
-        this.observer = new IntersectionController(this, config);
+        this.observer = new IntersectionController(this, {
+          callback: () => true,
+          ...config,
+        });
       }
 
       override update(props: PropertyValues) {
@@ -402,13 +406,15 @@ const canTest = () => {
 
   test('can observe changes when initialized after host connected', async () => {
     class TestFirstUpdated extends ReactiveElement {
-      observer!: IntersectionController;
+      observer!: IntersectionController<true>;
       observerValue: true | undefined = undefined;
       override firstUpdated() {
-        this.observer = new IntersectionController(this, {});
+        this.observer = new IntersectionController(this, {
+          callback: () => true,
+        });
       }
       override updated() {
-        this.observerValue = this.observer.value as typeof this.observerValue;
+        this.observerValue = this.observer.value;
       }
       resetObserverValue() {
         this.observer.value = this.observerValue = undefined;
@@ -438,16 +444,17 @@ const canTest = () => {
     const d = document.createElement('div');
     container.appendChild(d);
     class A extends ReactiveElement {
-      observer!: IntersectionController;
+      observer!: IntersectionController<true>;
       observerValue: true | undefined = undefined;
       override firstUpdated() {
         this.observer = new IntersectionController(this, {
           target: d,
           skipInitial: true,
+          callback: () => true,
         });
       }
       override updated() {
-        this.observerValue = this.observer.value as typeof this.observerValue;
+        this.observerValue = this.observer.value;
       }
       resetObserverValue() {
         this.observer.value = this.observerValue = undefined;
@@ -468,4 +475,38 @@ const canTest = () => {
     await intersectionComplete();
     assert.isTrue(el.observerValue);
   });
+
+  test('IntersectionController<T> type-checks', async () => {
+    // This test only checks compile-type behavior. There are no runtime checks.
+    const el = await getTestElement();
+    const A = new IntersectionController<number>(el, {
+      // @ts-expect-error Type 'string' is not assignable to type 'number'
+      callback: () => '',
+    });
+    if (A) {
+      // Suppress no-unused-vars warnings
+    }
+
+    const B = new IntersectionController(el, {
+      callback: () => '',
+    });
+    // @ts-expect-error Type 'number' is not assignable to type 'string'.
+    B.value = 2;
+
+    const C = new IntersectionController(
+      el,
+      {} as IntersectionController<string>
+    );
+    // @ts-expect-error Type 'number' is not assignable to type 'string'.
+    C.value = 3;
+
+    const narrowTypeCb: IntersectionValueCallback<string | null> = () => '';
+    const D = new IntersectionController(el, {callback: narrowTypeCb});
+
+    D.value = null;
+    D.value = undefined;
+    D.value = '';
+    // @ts-expect-error Type 'number' is not assignable to type 'string'
+    D.value = 3;
+  });
 });