Merge 791065a into 25bee8c

.changeset/fluffy-papers-sit.md

@@ -0,0 +1,16 @@
+---
+"@patternfly/pfe-core": minor
+---
+**Decorators**: Added `@listen`. Use it to attach element event listeners to
+class methods.
+
+```ts
+@customElement('custom-input')
+class CustomInput extends LitElement {
+  @property({ type: Boolean }) dirty = false;
+  @listen('keyup', { once: true })
+  protected onKeyup() {
+    this.dirty = true;
+  }
+}
+```

.changeset/poor-years-hug.md

@@ -0,0 +1,28 @@
+---
+"@patternfly/pfe-core": major
+---
+**Decorators**: Added `@observes`. Use it to add property change callback by 
+decorating them with the name of the property to observe
+
+```ts
+@customElement('custom-button')
+class CustomButton extends LitElement {
+  #internals = this.attachInternals();
+
+  @property({ type: Boolean }) disabled = false;
+
+  @observes('disabled')
+  protected disabledChanged() {
+    this.#internals.ariaDisabled =
+        this.disabled ? 'true'
+      : this.ariaDisabled ?? 'false';
+  }
+}
+```
+
+Breaking change: This commit makes some changes to the internal APIs of the
+pre-existing `@observed` observer, most notably it changes the constructor
+signature of the `PropertyObserverController` and associated functions. Most
+users should not have to make any changes, but if you directly import and use
+those functions, check the commit log to see how to update your call sites.
+

core/pfe-core/controllers/property-observer-controller.ts

@@ -1,52 +1,59 @@
 import type { ReactiveController, ReactiveElement } from 'lit';
 
-export const observedController: unique symbol = Symbol('observed properties controller');
-
-export type ChangeCallback<T = ReactiveElement> = (
+export type ChangeCallback<T extends ReactiveElement, V = T[keyof T]> = (
   this: T,
-  old?: T[keyof T],
-  newV?: T[keyof T],
+  old?: V,
+  newV?: V,
 ) => void;
 
-export type ChangeCallbackName = `_${string}Changed`;
-
-export type PropertyObserverHost<T> = T & Record<ChangeCallbackName, ChangeCallback<T>> & {
-  [observedController]: PropertyObserverController;
-};
-
-/** This controller holds a cache of observed property values which were set before the element updated */
-export class PropertyObserverController implements ReactiveController {
-  private static hosts = new WeakMap<HTMLElement, PropertyObserverController>();
-
-  private values = new Map<string, [methodName: string, values: [unknown, unknown]]>();
+export interface PropertyObserverOptions<T extends ReactiveElement> {
+  propertyName: string & keyof T;
+  callback: ChangeCallback<T>;
+  waitFor?: 'connected' | 'updated' | 'firstUpdated';
+}
 
-  private delete(key: string) {
-    this.values.delete(key);
-  }
+export class PropertyObserverController<
+  T extends ReactiveElement
+> implements ReactiveController {
+  private oldVal: T[keyof T];
 
-  constructor(private host: ReactiveElement) {
-    if (PropertyObserverController.hosts.get(host)) {
-      return PropertyObserverController.hosts.get(host) as PropertyObserverController;
-    }
-    host.addController(this);
-    (host as PropertyObserverHost<ReactiveElement>)[observedController] = this;
+  constructor(
+    private host: T,
+    private options: PropertyObserverOptions<T>
+  ) {
+    this.oldVal = host[options.propertyName];
   }
 
   /** Set any cached valued accumulated between constructor and connectedCallback */
-  hostUpdate(): void {
-    for (const [key, [methodName, [oldVal, newVal]]] of this.values) {
-      // @ts-expect-error: be cool, typescript
-      this.host[methodName as keyof ReactiveElement]?.(oldVal, newVal);
-      this.delete(key);
+  async hostUpdate(): Promise<void> {
+    const { oldVal, options: { waitFor, propertyName, callback } } = this;
+    if (!callback) {
+      throw new Error(`no callback for ${propertyName}`);
     }
-  }
-
-  /** Once the element has updated, we no longer need this controller, so we remove it */
-  hostUpdated(): void {
-    this.host.removeController(this);
-  }
-
-  cache(key: string, methodName: string, ...vals: [unknown, unknown]): void {
-    this.values.set(key, [methodName, vals]);
+    const newVal = this.host[propertyName];
+    this.oldVal = newVal;
+    if (newVal !== oldVal) {
+      switch (waitFor) {
+        case 'connected':
+          if (!this.host.isConnected) {
+            const origConnected = this.host.connectedCallback;
+            await new Promise<void>(resolve => {
+              this.host.connectedCallback = function() {
+                resolve(origConnected?.call(this));
+              };
+            });
+          }
+          break;
+        case 'firstUpdated':
+          if (!this.host.hasUpdated) {
+            await this.host.updateComplete;
+          }
+          break;
+        case 'updated':
+          await this.host.updateComplete;
+          break;
+      }
+    }
+    callback.call(this.host, oldVal as T[keyof T], newVal);
   }
 }

core/pfe-core/decorators.ts

@@ -2,6 +2,8 @@ export * from './decorators/bound.js';
 export * from './decorators/cascades.js';
 export * from './decorators/deprecation.js';
 export * from './decorators/initializer.js';
+export * from './decorators/listen.js';
 export * from './decorators/observed.js';
+export * from './decorators/observes.js';
 export * from './decorators/time.js';
 export * from './decorators/trace.js';

core/pfe-core/decorators/listen.ts

@@ -0,0 +1,29 @@
+import type { LitElement } from 'lit';
+
+/**
+ * Listens for a given event on the custom element.
+ * equivalent to calling `this.addEventListener` in the constructor
+ * @param type event type e.g. `click`
+ * @param options event listener options object e.g. `{ passive: true }`
+ */
+export function listen<P extends LitElement>(
+  type: keyof HTMLElementEventMap,
+  options?: EventListenerOptions,
+) {
+  return function(
+    proto: LitElement,
+    methodName: string,
+  ): void {
+    const origConnected = proto.connectedCallback;
+    const origDisconnected = proto.disconnectedCallback;
+    const listener = (proto as P)[methodName as keyof P] as EventListener;
+    proto.connectedCallback = function() {
+      origConnected?.call(this);
+      this.addEventListener(type, listener, options);
+    };
+    proto.disconnectedCallback = function() {
+      origDisconnected?.call(this);
+      this.removeEventListener(type, listener, options);
+    };
+  };
+}

core/pfe-core/decorators/observed.ts

@@ -1,14 +1,7 @@
 import type { ReactiveElement } from 'lit';
-import type {
-  ChangeCallback,
-  ChangeCallbackName,
-  PropertyObserverHost,
-} from '../controllers/property-observer-controller.js';
+import type { ChangeCallback } from '../controllers/property-observer-controller.js';
 
-import {
-  observedController,
-  PropertyObserverController,
-} from '../controllers/property-observer-controller.js';
+import { PropertyObserverController } from '../controllers/property-observer-controller.js';
 
 type TypedFieldDecorator<T> = (proto: T, key: string | keyof T) => void ;
 
@@ -35,66 +28,68 @@ type TypedFieldDecorator<T> = (proto: T, key: string | keyof T) => void ;
  *          @observed((oldVal, newVal) => console.log(`Size changed from ${oldVal} to ${newVal}`))
  *          ```
  */
+export function observed<T extends ReactiveElement, V>(
+  cb: ChangeCallback<T, V>,
+): TypedFieldDecorator<T>;
 export function observed<T extends ReactiveElement>(methodName: string): TypedFieldDecorator<T>;
-export function observed<T extends ReactiveElement>(cb: ChangeCallback<T>): TypedFieldDecorator<T>;
 export function observed<T extends ReactiveElement>(proto: T, key: string): void;
 // eslint-disable-next-line jsdoc/require-jsdoc
 export function observed<T extends ReactiveElement>(...as: any[]): void | TypedFieldDecorator<T> {
   if (as.length === 1) {
-    const [methodNameOrCallback] = as;
-    return function(proto, key) {
-      (proto.constructor as typeof ReactiveElement)
-          .addInitializer(x => new PropertyObserverController(x));
-      observeProperty(proto, key as string & keyof T, methodNameOrCallback);
-    };
+    const [methodNameOrCb] = as;
+    return configuredDecorator(methodNameOrCb);
   } else {
-    const [proto, key] = as;
-    (proto.constructor as typeof ReactiveElement)
-        .addInitializer(x => new PropertyObserverController(x));
-    observeProperty(proto, key);
+    return executeBareDecorator(...as as [T, string & keyof T]);
   }
 }
 
 /**
- * Creates an observer on a field
- * @param proto
- * @param key
- * @param callbackOrMethod
+ * @param proto element prototype
+ * @param propertyName propertyName
+ * @example ```typescript
+ *          @observed @property() foo?: string;
+ *          ```
  */
-export function observeProperty<T extends ReactiveElement>(
-  proto: T,
-  key: string & keyof T,
-  callbackOrMethod?: ChangeCallback<T>
-): void {
-  const descriptor = Object.getOwnPropertyDescriptor(proto, key);
-  Object.defineProperty(proto, key, {
-    ...descriptor,
-    configurable: true,
-    set(this: PropertyObserverHost<T>, newVal: T[keyof T]) {
-      const oldVal = this[key as keyof T];
-      // first, call any pre-existing setters, e.g. `@property`
-      descriptor?.set?.call(this, newVal);
+function executeBareDecorator<T extends ReactiveElement>(proto: T, propertyName: string & keyof T) {
+  const klass = proto.constructor as typeof ReactiveElement;
+  klass.addInitializer(x => initialize(
+    x as T,
+    propertyName,
+    x[`_${propertyName}Changed` as keyof typeof x] as ChangeCallback<T>,
+  ));
+}
 
-      // if the user passed a callback, call it
-      // e.g. `@observed((_, newVal) => console.log(newVal))`
-      // safe to call before connectedCallback, because it's impossible to get a `this` ref.
-      if (typeof callbackOrMethod === 'function') {
-        callbackOrMethod.call(this, oldVal, newVal);
-      } else {
-        // if the user passed a string method name, call it on `this`
-        // e.g. `@observed('_renderOptions')`
-        // otherwise, use a default method name e.g. `_fooChanged`
-        const actualMethodName = callbackOrMethod || `_${key}Changed`;
+/**
+ * @param methodNameOrCb string name of callback or function
+ * @example ```typescript
+ *          @observed('_myCallback') @property() foo?: string;
+ *          @observed((old) => console.log(old)) @property() bar?: string;
+ *          ```
+ */
+function configuredDecorator<T extends ReactiveElement>(
+  methodNameOrCb: string | ChangeCallback<T>,
+): TypedFieldDecorator<T> {
+  return function(proto, key) {
+    const propertyName = key as string & keyof T;
+    const klass = proto.constructor as typeof ReactiveElement;
+    if (typeof methodNameOrCb === 'function') {
+      const callback = methodNameOrCb;
+      klass.addInitializer(x => initialize(x as T, propertyName, callback));
+    } else {
+      klass.addInitializer(x => initialize(
+        x as T,
+        propertyName,
+        x[methodNameOrCb as keyof ReactiveElement] as ChangeCallback<T>,
+      ));
+    }
+  };
+}
 
-        // if the component has already connected to the DOM, run the callback
-        // otherwise, If the component has not yet connected to the DOM,
-        // cache the old and new values. See PropertyObserverController above
-        if (this.hasUpdated) {
-          this[actualMethodName as ChangeCallbackName]?.(oldVal, newVal);
-        } else {
-          this[observedController].cache(key as string, actualMethodName, oldVal, newVal);
-        }
-      }
-    },
-  });
+function initialize<T extends ReactiveElement>(
+  instance: T,
+  propertyName: string & keyof T,
+  callback: ChangeCallback<T>,
+) {
+  const controller = new PropertyObserverController<T>(instance as T, { propertyName, callback });
+  instance.addController(controller);
 }

core/pfe-core/decorators/observes.ts

@@ -0,0 +1,40 @@
+import type { ReactiveElement } from 'lit';
+
+import {
+  PropertyObserverController,
+  type ChangeCallback,
+  type PropertyObserverOptions,
+} from '@patternfly/pfe-core/controllers/property-observer-controller.js';
+
+/**
+ * Observes changes on the given property and calls the decorated method
+ * with the old and new values when it changes. In cases where the decorated method
+ * needs to access uninitialized class fields, You may need to wait for the element to connect
+ * before running your effects. In that case, you can optionally specify which
+ * lifecycle state to wait for. e.g.:
+ * - `waitFor: 'firstUpdate'` waits until the first update cycle has completed
+ * - `waitFor: 'updated'` waits until the next update cycle has completed
+ * - `waitFor: 'connected'` waits until the element connects
+ * @param propertyName property to react to
+ * @param [options] options including lifecycle to wait on.
+ */
+export function observes<T extends ReactiveElement>(
+  propertyName: string & keyof T,
+  options?: Partial<Pick<PropertyObserverOptions<T>, 'waitFor'>>,
+) {
+  return function(proto: T, methodName: string): void {
+    const callback = proto[methodName as keyof T] as ChangeCallback<T>;
+    if (typeof callback !== 'function') {
+      throw new Error('@observes must decorate a class method');
+    }
+    const klass = proto.constructor as typeof ReactiveElement;
+    klass.addInitializer(instance => {
+      instance.addController(new PropertyObserverController(instance as T, {
+        ...options,
+        propertyName,
+        callback,
+      }));
+    });
+  };
+}
+