tc39/proposal-decorators

setMetadata and inheritance

Closed this issue · 9 comments

pabloalmunia commented

In the current proposal, we can read:

Finally, the metadata object itself inherits from the parent class' metadata object if it exists, the public object inherits from the parent's public object if it exists.

This is a simple case:

const MY_SYMBOL = Symbol()
function meta(k, v) {
  return function(value, context) {
    context.setMetadata(MY_SYMBOL, {[k]:v});
  }
}


class P {
  @metadata('n', 1)
  a = 1;
  @metadata('n', 2)
  b = true;
}

class C extends P {
  @metadata('n', 3)
  a = 2;
}

P.prototype[Symbol.metadata] = {}
C.prototype[Symbol.metadata] = {}

const MY_SYMBOL = Symbol();

console.log(P.prototype[Symbol.metadata][MY_SYMBOL])
// {
//   public: {
//     a: {n: 1},
//     b: {n: 2}
//   },
//   private: []
// }

console.log(C.prototype[Symbol.metadata][MY_SYMBOL])
//
// => Option 1
// {
//   public: {
//     a: {n: 3}
//   },
//   private: []
// }
//
// => Option 2
// {
//   public: {
//     a: {n: 3}
//     b: {n: 2}
//   },
//   private: []
// }

As you can see, we have two options for the child class metadata.

  • The one option is the result of overwrite the P.prototype[Symbol.metadata][MY_SYMBOL] with C.prototype[Symbol.metadata][MY_SYMBOL].
  • The second options is the result is a merge from P.prototype[Symbol.metadata][MY_SYMBOL] and C.prototype[Symbol.metadata][MY_SYMBOL]. We believe this is the right outcome.

In our view, this kind of merge is only possible if the transpiler implements the .prototype[Symbol.metadata][MY_SYMBOL] as a Proxy and goes through the prototype chain, merging the content for every metadata. It's is not possible to implement with getters.

For example, if we get C.prototype[Symbol.metadata][MY_SYMBOL].b the Proxy trap this call, search the b into the prototype chain metadata and return the first value.

Remember, the prototype chain must be change in with setPrototypeOf() at any time.

Please, share with us your point of view about the best way to transpile this behavior.

pzuraq commented

Per the spec, after the class is defined, basically what should happen is:

let parentMetadata = P.prototype[Symbol.metadata]
let childMetadata = C.prototype[Symbol.metadata]

Object.setPrototypeOf(C, P);

for (let key of Object.getOwnPropertySymbols(C)) {
  let parentMetadataForKey = parentMetadata[key];
  let childMetadataForKey = childMetadata[key];

  if (!parentMetadataForKey) continue;
  
  Object.setPrototypeOf(childMetadataForKey, parentMetadataForKey);

  if (childMetadataForKey.public && parentMetadatForKey.public) {
    Object.setPrototypeOf(childMetadataForKey.public, parentMetadataForKey.public);
  }
}

This implementation results in natural prototype inheritance giving us the correct metadata for the subclass, including metadata from the parent class which has not been shadowed. Metadata which has been shadowed can still be accessed by traversing the prototype chain.

The reason that private does not follow this same scheme is that private elements do not have a meaningful name, and also cannot be shadowed. So, they are represented as an array, and the inheritance is done via the getter concatenating dynamically.

pabloalmunia commented

This is a good solution, but it is completely static. If a decorator or other code changes the prototype chain, this solution returns the original metadata, but not the current metadata. The class' prototype chain can be changed with setPrototypeOf() at any time and the solution of linking the metadata prototype would use only the prototype chain at the time of defining the metadata.

pzuraq commented

Correct, that is the expected requirement currently if you are changing the prototype chain dynamically. Updating the parent and child metadata prototypes is a manual step. The only reason it's not for private metadata is because it would be pretty tricky to do so.

Maybe it should be required for private metadata though. You could do childMetadata.filter((m) => !oldParentMetadata.includes(m)).concat(newParentMetadata), which is a bit annoying, but not much moreso than with manually updating the prototypes.

pabloalmunia commented

Other question about setMEtada and multiple inheritance levels:

In this case, the class B has not metadata, but A, C, and D has metadata:

const KEY = Symbol();
function metadata(data) {
  return function (value, context) {
    context.setMetadata(KEY, data);
  };
}

class A {
  @metadata(10)
  a() {}
}

class B extends A {
  b() {}
}

class C extends B {
  @metadata(30)
  c() {}
}

class D extends C {
  @metadata(40)
  d() {}
}

console.log(D.prototype[Symbol.metadata][KEY]);
// {
//   public : {
//     a : 10,
//     c : 30,
//     d : 40
//   }
//   private : []
// }

console.log(B.prototype[Symbol.metadata][KEY]);
// Option 1:
// undefined
//
// Option 2:
// {
//   public : {
//     a: 10
//   }
//   private : []
// }

With option 1, class B has not metadata, and C metadata must be linked directly with A.

With option 2, class B has metadata inherited from A.

In other words, does an undecorated class with no metadata have metadata for its inheritance or not?

pabloalmunia commented

Well, we have now solved our last issue. We have implemented three prototype chain, one for the Symbol.metadata objects, one for each of the metadata keys (which are also Symbol), and one for .public. This way the .public properties of each of the keys are not overshadowed.

It's a bit complicated to explain clearly in words (my English is limited), but maybe you can understand this code better:

const KEY = Symbol();

function metadata(data) {
  return function (value, context) {
    context.setMetadata(KEY, data);
  };
}

class A {
  @metadata(10)
  a() {}
}

console.assert(A.prototype[Symbol.metadata][KEY].public.a === 10);

class B extends A {
  b() {}
}

// B.prototype don't have an own property Symbol.metadata,
// but can access through its prototype chain
console.assert(B.prototype[Symbol.metadata][KEY].public.a === 10);

class C extends B {
  @metadata(30)
  c() {}
}

console.assert(C.prototype[Symbol.metadata][KEY].public.a === 10);
console.assert(C.prototype[Symbol.metadata][KEY].public.c === 30);

class D extends C {
  @metadata(40)
  d() {}
}

console.assert(D.prototype[Symbol.metadata][KEY].public.a === 10);
console.assert(D.prototype[Symbol.metadata][KEY].public.c === 30);
console.assert(D.prototype[Symbol.metadata][KEY].public.d === 40);

// Common prototype chain
console.assert(Object.getPrototypeOf(B) === A)
console.assert(Object.getPrototypeOf(C) === B)
console.assert(Object.getPrototypeOf(D) === C)

// B.prototype don't have an own property Symbol.metadata, as a result, this is wrong
// console.assert(Object.getPrototypeOf(B.prototype[Symbol.metadata]) === A.prototype[Symbol.metadata])

// Symbol.metadata prototype chain
console.assert(Object.getPrototypeOf(C.prototype[Symbol.metadata]) === A.prototype[Symbol.metadata])
console.assert(Object.getPrototypeOf(D.prototype[Symbol.metadata]) === C.prototype[Symbol.metadata])

// KEY prototype chain
console.assert(Object.getPrototypeOf(C.prototype[Symbol.metadata][KEY]) === A.prototype[Symbol.metadata][KEY])
console.assert(Object.getPrototypeOf(D.prototype[Symbol.metadata][KEY]) === C.prototype[Symbol.metadata][KEY])

// [KEY].public prototype chain
console.assert(Object.getPrototypeOf(C.prototype[Symbol.metadata][KEY].public) === A.prototype[Symbol.metadata][KEY].public)
console.assert(Object.getPrototypeOf(D.prototype[Symbol.metadata][KEY].public) === C.prototype[Symbol.metadata][KEY].public)
pabloalmunia commented

When a class inheritance has a level without metadata, the prototype metadata chain is problematic and we need to jump this class without metadata. It is very problematic. Please. @pzuraq check this case with the previous example. The class C metadata (KEY and public) is linked with class A metadata without class B. Is it wrong?

pzuraq commented

@pabloalmunia this is how the spec is meant to operate, if a class is missing metadata then it skips that level and uses the superclass's metadata. If the superclass does not have metadata, it does not inherit. I'm not sure what the problem is here?

pabloalmunia commented

Thanks @pzuraq, the solution is very elegant. In practice, we have three nested objects in the metadata: [Symbol.metadata], [KEY] and public.

Symbol.metadata = Symbol();
const KEY = Symbol();

class A {
  p;
}
A.prototype[Symbol.metadata] = Object.create(null);
A.prototype[Symbol.metadata][KEY] = Object.create(null);
A.prototype[Symbol.metadata][KEY].public = Object.create(null);
A.prototype[Symbol.metadata][KEY].public.p = 10;

Now we add two classes that inherit from A at two levels.

class B extends A {
  q;
}

class C extends B {
  z;
}

When defining new metadata, we only need to create the necessary prototype chain for [Symbol.metada], [KEY] and .public.

C.prototype[Symbol.metadata] = Object.create(Object.getPrototypeOf(C).prototype[Symbol.metadata]);
C.prototype[Symbol.metadata][KEY] = Object.create(Object.getPrototypeOf(C).prototype[Symbol.metadata][KEY]);
C.prototype[Symbol.metadata][KEY].public = Object.create(Object.getPrototypeOf(C).prototype[Symbol.metadata][KEY].public);
C.prototype[Symbol.metadata][KEY].public.z = 20

As a result, the class C has access to self metadata and the prototype chain metadata.

pabloalmunia commented

We rewrite setMetadata() and this is the current transpilation:

const KEY = Symbol();

function metadata(data) {
  return function(value, context) {
    context.setMetadata(KEY, data);
  };
}

if (!Symbol.metadata) {
  Symbol.metadata = Symbol("Symbol.metadata");
}

const __metadataPrivate = new WeakMap();

function __PrepareMetadata(base, kind, property) {
  const createObjectWithPrototype = (obj, key) => Object.hasOwnProperty.call(obj, key) ? obj[key] : Object.create(obj[key] || null);
  return {
    getMetadata(key) {
      if (base[Symbol.metadata] && base[Symbol.metadata][key] && typeof base[Symbol.metadata][key][kind] !== "undefined") {
        return kind === "public" ? base[Symbol.metadata][key].public[property] : base[Symbol.metadata][key][kind];
      }
    },
    setMetadata(key, value) {
      if (typeof key !== "symbol") {
        throw new TypeError("the key must be a Symbol");
      }
      base[Symbol.metadata] = createObjectWithPrototype(base, Symbol.metadata);
      base[Symbol.metadata][key] = createObjectWithPrototype(base[Symbol.metadata], key);
      base[Symbol.metadata][key].public = createObjectWithPrototype(base[Symbol.metadata][key], "public");
      if (!Object.hasOwnProperty.call(base[Symbol.metadata][key], "private")) {
        Object.defineProperty(base[Symbol.metadata][key], "private", {
          get() {
            return (__metadataPrivate.get(base[Symbol.metadata][key]) || []).concat(Object.getPrototypeOf(base[Symbol.metadata][key])?.private || []);
          }
        });
      }
      if (kind === "public") {
        base[Symbol.metadata][key].public[property] = value;
      } else if (kind === "private") {
        if (!__metadataPrivate.has(base[Symbol.metadata][key])) {
          __metadataPrivate.set(base[Symbol.metadata][key], []);
        }
        __metadataPrivate.get(base[Symbol.metadata][key]).push(value);
      } else if (kind === "constructor") {
        base[Symbol.metadata][key].constructor = value;
      }
    }
  };
}

class A {
  a() {}
}

A.prototype.a = metadata(10)(A.prototype.a, {
  kind: "method",
  name: "a",
  isStatic: false,
  isPrivate: false,
  ...__PrepareMetadata(A.prototype, "public", "a")
}) ?? A.prototype.a;