tc39 / proposal-decorators

Decorators for ES6 classes
https://arai-a.github.io/ecma262-compare/?pr=2417
2.76k stars 106 forks source link

Metadata inheritance #534

Closed fwolff closed 4 months ago

fwolff commented 4 months ago

Hello,

I posted an issue about the Typescript transpilation of decorators in case of fields overwriting: https://github.com/microsoft/TypeScript/issues/59081. In short, the metadata of an inherited class is constructed with a prototype inheritance of the parent’s metadata:

Object.create((_b = _classSuper[Symbol.metadata])

This is annoying when one wants to override the decorator value because it changes the parent’s class metadata (see my example in the TS issue).

Does the TC39 proposal say anything about that? Maybe this behavior should be clarified.

pzuraq commented 4 months ago

This is the intended behavior, the idea is that you can override values on the child by assigning own properties to metadata. This allows non-overridden fields to be inherited naturally, while still allowing easy overriding.

In your case, where the decorator is assigning an object to the property that you presumably manage, I suggest duplicating this inheritance chain:

function decor(value: boolean) {
    return function _(_: any, context: ClassFieldDecoratorContext<unknown, any>) {
        if (!Object.hasOwnProperty(context.metadata, context.name)) {
          context.metadata![context.name] = Object.create(context.metadata![context.name]);
        }

        (context.metadata![context.name] as any)['decor'] = value
    }
}

Or you could just overwrite the property entirely. This is a bit more verbose, but in general it follows a more common pattern with decorators that we've seen in general in the ecosystem.

fwolff commented 4 months ago

Ok, thank you for your quick reply! I came out with a solution close to yours, without duplicating the inheritance chain though. I didn't expect this "pitfall" when I started my experiments with decorators and it took me some time to figure out what was happening. But it is documented, sorry for the bother:

If the decorated class has a parent class, then the prototype of the metadata object is set to the metadata object of the superclass. This allows metadata to be inherited in a natural way, taking advantage of shadowing by default, mirroring class inheritance.

trusktr commented 2 months ago

Another way to do this, without having to create inheritance yourself, is to do it only with property naming:

function decor(value: boolean) {
    return function (_: any, context: ClassFieldDecoratorContext<unknown, any>) {
        context.metadata!['decor_' + context.name] = value
    }
}

That one is simpler, as it takes advantage of the built-in inheritance of context.metadata: a value for 'decor_' + context.name in the child class will automatically override the value for 'decor_' + context.name of the parent class.

If you have multiple decorators (supposedly that's why you had a separate object, f.e. {decorA: ..., decorB: ...}), you can just prefix with diffferent decorator names, so you have 'decorA_' + context.name, 'decorB_' + context.name, etc, where all of them will automatically use the existing inheritance. Basically string namespacing instead of object namespacing.