microsoft / microsoft/TypeScript

ClassDecorator type is inconsistent with Decorators proposal, needs type parameters

Open
#53,790 7 comments 0 reactions 1 assignee View on GitHub

@rbuckton is already working on this.

Since Apr 18, 2023.

Needs Investigation
Dominant language
Go
Stars
111k
Forks
14.3k
Avg merge
1d 19h
Merged PRs (30d)
117

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.