tc39 / proposal-decorators

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

Stage 3 cannot access the class for static method decorator #514

Closed cnwangjie closed 1 year ago

cnwangjie commented 1 year ago

In stage 2 the first parameter is the class of the method when using a method decorator for a static method. So I can instantiate the class in the decorator. In stage 3 I can only know the method is a static method but cannot get the class.

Considering following cases:

I have an eventHandler decorator that is a helper to register the handler of app events.

I implemented it by following code.

export const eventHandler = <T extends AppEventTarget<any>>(
  event: T,
): MethodDecorator => {
  return (target, _propertyKey, descriptor) => {
    const originalMethod: any = descriptor.value
    const isStaticMethod = target instanceof Function
    const cls = isStaticMethod ? target : target.constructor
    event.registerHandler(async (ctx, payload) => {
      const instance = new (cls as Constructor)(ctx)
      const args = isStaticMethod ? [ctx, payload] : [payload]
      await originalMethod.apply(instance, args)
    })
  }
}

And I can use it on methods that are static or not.


 export class A {
   @eventHandler(events.SomeTask)
   async a(payload: AppEventPayload<typeof events.SomeTask>) {
      // do some task
   } 
   @eventHandler(events.SomeTask)
   static async b(ctx: Context, payload: AppEventPayload<typeof events.SomeTask>) {
      // do some task
   }
 } 

But in stage 3 I cannot get the class.


export const eventHandler = <
  T extends AppEventTarget<any>,
  This extends BaseService,
  U = EventHandler<T, This>,
>(
  event: T,
) => {
  return (target: U, context: ClassMethodDecoratorContext) => {
    const cls = context.static ? /* How can I get class here */ : target
    event.registerHandler(async (ctx, payload) => {
      const instance = new (cls as Constructor)(ctx)
      const args = isStaticMethod ? [ctx, payload] : [payload]
      await originalMethod.apply(instance, args)
    })
  }
}
pzuraq commented 1 year ago

While you cannot access the class during decoration, you can access it during initialization. This would look like the following:

export const eventHandler = <T extends AppEventTarget<any>>(
  event: T,
) => {
  return (_, { access, static: isStatic, addInitializer }) => {
    addInitializer(function() {
      const cls = isStatic ? this : this.constructor

      event.registerHandler(async (ctx, payload) => {
        const instance = new (cls as Constructor)(ctx)
        const args = isStatic ? [ctx, payload] : [payload]
        await access.get().apply(instance, args)
      })
    });
  }
}

Note that addInitializer will run when the class is defined for static methods, but it will only run when the class is initialized for instance methods. So the while the original logic for this code would work for non-static methods, this new version would only work for static methods, until you actually create an instance. You could add an assertion that requires users to use static to avoid this.

trusktr commented 1 year ago

Besides using an initializer for static members, you can access the class in any decorator by sharing some state with a class decorator (later this will be easier with the decorator metadata add-on proposal), but note that if you modify the class shape (f.e. the instance or the class prototype) you may be undoing optimizations that the engine may have put in place:

const decoratedProps = []
const handledClasses = new Set()

function classDeco(decoratedClass) {
  // Do something with decoratedProps, f.e. return a class that deletes own fields, and converts them into getters/setters (without using the new accessor keyword!)

  const props = [...decoratedProps]
  decoratedProps = []

  return class extends decoratedClass {
    constructor(...args) {
      super(...args)

      if (!handledClasses.has(decoratedClass)) {
        handledClasses.add(decoratedClass)

        for (const prop of props) {
          const initialValue = this[prop]
          delete this[prop]
          createGetterSetter(decoratedClass.prototype, prop, initialValue)
        }
      }
    }
  }
}

function fieldDeco(_, { name }) {
  decoratedProps.push(name)
}

@classDeco
class Mine {
  @fieldDeco foo = 123 // Look ma, no accessor!
}

(Untested code, but you get the idea!)

Marsup commented 1 year ago

Hi,

Just bringing my 2c to the issue, I'm currently in the process of migrating from legacy TypeScript decorators to the new spec, and this is indeed quite weird not to have this. One of our usage is a cache decorator that composes class name and function name to build a part of its cache key, the finalized API forces me to use a pattern such as this:

function cache() {
  return function(target, context) {
    let cachePrefix = '';

    context.addInitializer(function () {
      const className = context.static ? this.name : this.constructor.name;
      cachePrefix = `${className}:${target.name}`;
    });

    return function handleCache(...args) {
      const cacheKey = `${cachePrefix}:${computeKeyFromArgs(args)}`;
      ...
      return result;
    }
  }
}

The previous version allowed the cachePrefix to be pre-computed in a constant and was IMHO more idiomatic than this version. Am I doing it wrong or is this expected and unavoidable?