microsoft/TypeScript

ClassDecorator type is inconsistent with Decorators proposal, needs type parameters

ajvincent opened this issue ยท 7 comments

Bug Report

๐Ÿ”Ž Search Terms

classdecorator
decorator context

๐Ÿ•— Version & Regression Information

  • This is the behavior in every version I tried (version 5.0+), and I reviewed the FAQ for entries about decorators.
  • I was unable to test this on prior versions because proper ECMAScript decorators only became available in TypeScript 5.0.

โฏ Playground Link

Playground Link

๐Ÿ’ป Code

class BaseClass {
}

const VoidClassDecorator = function(
    baseClass: typeof BaseClass,
    context: ClassDecoratorContext,
) : typeof BaseClass
{
    void(context);
    return baseClass;
}

@VoidClassDecorator
class UntypedDecorated extends BaseClass {

}

const TypedVoidClassDecorator: ClassDecorator = VoidClassDecorator;

@TypedVoidClassDecorator
class TypedDecorated extends BaseClass {
}

๐Ÿ™ Actual behavior

Two errors:

Type '(baseClass: typeof BaseClass, context: ClassDecoratorContext<abstract new (...args: any) => any>) => typeof BaseClass' is not assignable to type 'ClassDecorator'.
  Target signature provides too few arguments. Expected 2 or more, but got 1.

Unable to resolve signature of class decorator when called as an expression.
  The runtime will invoke the decorator with 2 arguments, but the decorator expects 1.

The only difference between TypedVoidClassDecorator and VoidClassDecorator is the application of the ClassDecorator type, which doesn't take the context argument current ECMAScript decorators may use.

๐Ÿ™‚ Expected behavior

No errors.

I would suggest a type taking three type parameters: the base class, a boolean flag for returning the original class type or returning void, and an arguments type which changes the return result.

import { Class } from "type-fest";

/* eslint-disable @typescript-eslint/no-explicit-any */
export type ClassDecoratorFunction<
  BaseClassType extends Class<unknown>,
  ReturnsModified extends boolean,
  Arguments extends any[] | false
> = 
    Arguments extends any[] ? 
    (...args: Arguments) => ClassDecoratorFunction<BaseClassType, ReturnsModified, false> :
    (
        baseClass: BaseClassType,
        context: ClassDecoratorContext,
    ) => (ReturnsModified extends true ? BaseClassType : void);

Playground Link, with fixes for earlier bugs

Update: fixing the above playground code!

This might require deprecating ye olde ClassDecorator type.

For people like me who are thinking about mix-ins ๐Ÿ˜ˆ , you might want to have ReturnsModified extends boolean | BaseClassType, so the user can specify a subclass type.

Yes, I know class decorators in TypeScript assert the same type as the pre-decorated class. I can (and have done) work around that.

ClassDecorator type, which doesn't take the context argument current ECMAScript decorators may use.

Isn't that type from legacy experimental decorators that follow old draft? At least in your playground it's already from lib.decorators.legacy.d.ts so it shouldn't be used already if you intent to use recent decorators proposal.

Wonder if it's possible to mark that type as @deprecated when experimentalDecorators: false

Please note I make no claims about the other categories of ECMAScript decorators. I have not explored their use yet.

Hello @ajvincent!

About #53790 (comment),
Did you built ClassDecoratorFunction yourself?

It is concerning #54338

Hello @ajvincent!

About #53790 (comment), Did you built ClassDecoratorFunction yourself?

It is concerning #54338

I created a type with that name (EDIT: buggy, see below for corrected type)

I am not aware of it being used anywhere else.

There was a bug in my type above. It should've said true extends ReturnsModified ? BaseClassType : where it says ReturnsModified extends true ? BaseClassType :.

Fixed permalink: https://github.com/ajvincent/es-membrane/blob/9bfd0d634f2d354f9c40dbc69d93df7a4985035b/_01_stage_utilities/source/types/ClassDecoratorFunction.d.mts

There was a bug in my type above. It should've said true extends ReturnsModified ? BaseClassType : where it says ReturnsModified extends true ? BaseClassType :.

Fixed permalink: https://github.com/ajvincent/es-membrane/blob/9bfd0d634f2d354f9c40dbc69d93df7a4985035b/_01_stage_utilities/source/types/ClassDecoratorFunction.d.mts

ajvincent/es-membrane@9bfd0d6