Potential issues with conditional metadata and mutability

[jridgewell]

Given the following case's class structure:

// addMetadata doesn't do anything, I just want the @@metadata added
@addMetadata
class SuperClass {}

// Notice SubClass doesn't have a decorator, so it doesn't have @@metadata
class SubClass extends SuperClass {}

Because of the conditional semantics, the SubClass will not have the @@metadata object. And because the subclass's constructor's __proto__ will be set to SuperClass, accessing SubClass[@@metadata] will actually return SuperClass[@@metadata].

This can cause some potential bugs if the metadata of the class is mutated after the fact:

function doWorkWithMetadata(klass) {
  const metadata = klass[Symbol.metadata];
  if (metadata.__workDone) return;

  // do some work on metadata
  console.log(klass);

  metadata.__workDone = true;
}

doWorkWithMetadata(SubClass);
doWorkWithMetadata(SuperClass);

This will only "do work" on the subclass. Because the mutation will affect both the subclass and superclass, the superclass is skipped.

[ljharb]

Is that a bug, or is that actually correct behavior because the superclass would presumably benefit from that work being done and thus correctly indicate it was done?

[bakkot]

It's definitely a bug in at least some cases - information derived from the subclass may not be true of the superclass.

[hax]

How modifying a class metadata causing all "sibling" subclasses metadata changed would be "correct behavior"?

[hax]

I suppose conditional creating metadata object is mainly for perf, but consider this footgun I feel we should always create metadata for subclasses. I guess engines could optimize such thing, only really create it when first access or even when first try to mutate it.

[pzuraq]

Typically state is not tied directly to the metadata object, it's tied to the instance of the class. For instance, something like MobX might track which properties are reactive and store state for them, but that state would either be keyed off the instance or in the case of static properties off the class itself.

I don't think conditional metadata will be a common footgun because of this, and when encountered it's trivial to work around. I feel like mutating metadata at all after definition is like mutating class prototypes in general, in that it should be avoided unless you really know what you're doing, but I can see why someone might reach for it if they are new to decorators and metadata.