@watch() shouldn't patch the prototype every time it's used.

[justinfagnani]

Describe the bug

@watch() currently patches the element prototype to override update() each time it's applied. Patching the prototype at all is less-then-ideal for performance, but creating a potentially long chain of calls will be worse. As decorators migrate towards standard decorators such patching will be impossible anyway, and there will be no other patching from experimental decorators, potentially making this patch (if it can be done) the only thing to change the class shape, slowing class initialization.

To fix this, I'd recommend making @watch() cooperate with a base class that allows watchers. The directive stores the information about the watcher in a cache, and the base class looks up the watchers to invoke them.

Here's some code from a similar utility I have:

type Observer = {
  method: (...args: unknown[]) => void;
  keys: Array<PropertyKey>;
};

const observersForClass = new WeakMap<
  DecoratorMetadataObject,
  Array<Observer>
>();

export class BaseElement extends ReactiveElement {
  override update(changedProperties: PropertyValues) {
    const meta = (this.constructor as typeof ElematicElement)[Symbol.metadata];
    const observers = observersForClass.get(meta);
    if (observers !== undefined) {
      for (const {keys, method} of observers) {
        if (keys.some((p) => changedProperties.has(p))) {
          const oldValues = keys.map((p) => changedProperties.get(p));
          method.apply(this, oldValues);
        }
      }
    }
    super.update(changedProperties);
  }
}

export function observe<T extends BaseElement>(...keys: Array<keyof T>) {
  return <C, V extends (this: C, ...args: any) => any>(
    method: V,
    context: ClassMethodDecoratorContext<C, V>,
  ) => {
    let observers = observersForClass.get(context.metadata);
    if (observers === undefined) {
      observersForClass.set(context.metadata, (observers = []));
    }
    observers.push({keys, method});
  };
}

This is a standard decorator, but it can be adapted to experimental decorators by using the class as the key into the observersForClass map.

One benefit of this approach is that the watcher run in definition order, which is probably more intuitive. Right now watchers run in reverse definition order because the later methods make the outer wrappers for update().

It would be a breaking change to require a supporting implementation of update(), but one could be provided by a mixin, or devs could extend ShoelaceElement.

To Reproduce

n/a

Demo

n/a

Screenshots

n/a

Browser / OS

all

Additional information

[KonnorRogers]

Oh my....first off thank you, second off...I'm really curious how many components are relying on the reverse order...

[theacodes]

Actually, @justinfagnani would this be something you'd be willing to add upstream to Lit? I'd be happy to do the work of creating the PR.