TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Implement the Stage 3 Decorators Proposal

Open rbuckton opened this issue 3 years ago • 50 comments

This implements support for the Stage 3 Decorators proposal targeting ESNext through ES5 (except where it depends on functionality not available in a specific target, such as WeakMaps for down-level private names).

The following items are not currently supported:

  • --emitDecoratorMetadata, as metadata is currently under discussion in https://github.com/tc39/proposal-decorator-metadata and has not yet reached Stage 3.
  • Decorators on a declare field.
  • Parameter decorators will not be supported until a follow-on proposal has been adopted and has advanced to Stage 3.
    • See https://github.com/tc39/proposal-decorators/issues/47 for additional information.
  • Decorators may not change the type of the member or class they decorate. If we decide to allow this capability, we will do so in a later PR. This is under discussion in the following two issues:
    • #50159
    • #49229

With that out of the way, the following items are what is supported, or is new or changed for Decorators support in the Stage 3 proposal:

  • The --experimentalDecorators flag will continue to opt-in to the legacy decorator support (which still continues to support --emitDecoratorMetadata and parameter decorators).
  • ES Decorators are now supported without the --experimentalDecorators flag.
  • 🆕 ES Decorators will be transformed when the target is less than ESNext (or at least, until such time as the proposal reaches Stage 4).
  • 🆕 ES Decorators now accept exactly two arguments: target and context:
    • target — A value representing the element being decorated:
      • Classes, Methods, get accessors, and set accessors: This will be the function for that element.
      • Auto-Accessor fields (i.e., accessor x): This will be an object with get and set properties.
      • Fields: This will always be undefined.
    • context — An object containing additional context information about the decorated element such as:
      • kind - The kind of element ("class", "method", "getter", "setter", "field", "accessor").
      • name - The name of the element (either a string or symbol).
      • private - Whether the element has a private name.
      • static - Whether the element was declared static.
      • access - An object with either a get property, a set property, or both, that is used to read and write to the underlying value on an object.
      • addInitializer - A function that can be called to register a callback that is evaluated either when the class is defined or when an instance is created:
        • For static member decorators, initializers run after class decorators have been applied but before static fields are initialized.
        • For Class Decorators, initializers run after all static initializers.
        • For non-static member decorators, initializers run in the constructor before all field initializers are evaluated.
  • 🆕 ES Decorators can decorate private fields.
  • 🆕 ES Decorators can decorate class expressions.
  • ‼️ ES Accessor Decorators (i.e., for get and set declarations) no longer receive the combined property descriptor. Instead, they receive the accessor function they decorate.
    • A stage 1 proposal that expands upon auto-accessors to allow you to decorate get/set pairs can be found at https://github.com/tc39/proposal-grouped-and-auto-accessors.
  • ‼️ ES Member Decorators (i.e., for accessors, fields, and methods) no longer have immediate access to the constructor/prototype the member is defined on.
    • If you need access to the class constructor from a static member, you can use:
      context.addInitializer(function() { this /*constructor reference*/ });
      
    • If you need access to the instance (not the prototype) from a non-static member, you can use:
      context.addInitializer(function() { this /*instance reference*/ });
      
    • Non-static members currently have no way to access the constructor or prototype during class definition.
    • This behavior is under discussion at https://github.com/tc39/proposal-decorators/issues/465.
  • ‼️ ES Member Decorators can no longer set the enumerable, configurable, or writable properties as they do not receive the property descriptor. You can partially achieve this via context.addInitializer, but with the caveat that initializers added by non-static member decorators will run during every instance construction.
  • When the name of the class is inferred from an assignment, we will now explicitly set the name of the class in some cases. This is not currently consistent in all cases and is only set when transforming native ES Decorators or class fields. While we generally have not strictly aligned with the ECMA-262 spec with respect to assigned names when downleveling classes and functions (sometimes your class will end up with an assigned name of class_1 or default_1), I opted to include this because name is one of the few keys available to a class decorator's context object, making it more important to support correctly.

Finally, I also added a diagnostic function to the emitter that will write out comments before each node to indicate which transformer created the node. This can be enabled via the annotateTransforms compiler option, but I would only recommend enabling it if you are working on the compiler itself (or potentially if you are writing custom transformers).

Fixes #48885

rbuckton avatar Sep 17 '22 23:09 rbuckton

The --experimentalDecorators flag will continue to opt-in to the legacy decorator support

At some point this flag should probably be aliased/renamed to legacyDecorators or something, since “experimental” tends to imply “bleeding edge” and I can imagine future people unfamiliar with TS’s history blindly enabling it thinking they’re opting into something new and shiny as opposed to what it actually is, old and crusty. :wink:

the thought process I’m imagining is essentially, “ooh, I like ES decorators, I wonder if this will give me even cooler decorator features…”

fatcerberus avatar Sep 18 '22 16:09 fatcerberus

At some point this flag should probably be aliased/renamed to legacyDecorators or something, since “experimental” tends to imply “bleeding edge” and I can imagine future people unfamiliar with TS’s history blindly enabling it thinking they’re opting into something new and shiny as opposed to what it actually is, old and crusty. 😉

Maybe aliased, but probably not renamed so as not to break existing consumers. Also, parameter decorators are still experimental.

rbuckton avatar Sep 19 '22 15:09 rbuckton

Also, parameter decorators are still experimental.

Yeah, my point was more that at some point we’re going to have a flag called “experimental” that opts into legacy behavior, and worse, legacy behavior that’s incompatible with the standard behavior that’ll be supported by default. It’s a weird state of affairs and I can definitely foresee the future GH issues “I enabled experimentalDecorators and all my existing decorators stopped working correctly, I thought this would just unlock additional features”

fatcerberus avatar Sep 19 '22 15:09 fatcerberus

I have 2 questions:

  1. Why Non-static members currently have no way to access the constructor or prototype during class definition?
  2. Class decorator return a non-constructor value (like { }) is useful. Could you implement it?

ruojianll avatar Sep 25 '22 11:09 ruojianll

  1. Why Non-static members currently have no way to access the constructor or prototype during class definition?

That is the current behavior of the proposal, but an alternative is being discussed in https://github.com/tc39/proposal-decorators/issues/465.

  1. Class decorator return a non-constructor value (like { }) is useful. Could you implement it?

Class decorators can only return functions. You are welcome to open an issue at https://github.com/tc39/proposal-decorators if you believe this should be changed.

rbuckton avatar Sep 25 '22 17:09 rbuckton

Private static elements should now be supported as of the latest commit.

rbuckton avatar Sep 28 '22 21:09 rbuckton

@rbuckton Hey, I'm looking forward to this landing, any idea when that might happen? I imagine once this PR merges, the next nightly might have this?

AaronFriel avatar Oct 17 '22 18:10 AaronFriel

@rbuckton Hey, I'm looking forward to this landing, any idea when that might happen? I imagine once this PR merges, the next nightly might have this?

Yes, it should be available in the nightly following the merge, though that won't be until after we ship the 4.9 RC and main is ready for post-4.9 work.

rbuckton avatar Oct 17 '22 19:10 rbuckton

I'm thinking of pulling the --annotateTransforms diagnostics switch into its own PR and instead emitting the annotations as some type of source map extension. That plus a VS Code extension to add a hover or editor decoration might make for a good hackathon/side project sometime. As it stands, the comments it emits to mark transform nodes are helpful but make the output unreadable so its utility is a mixed bag.

rbuckton avatar Oct 25 '22 01:10 rbuckton

I'm thinking of pulling the --annotateTransforms diagnostics switch into its own PR ...

This is done, the --annotateTransforms diagnostics switch has been moved to #51307 instead.

rbuckton avatar Oct 25 '22 23:10 rbuckton

@typescript-bot pack this

DanielRosenwasser avatar Oct 27 '22 21:10 DanielRosenwasser

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 49dbc72074572c7f3e907f8bb02a11dff413325b. You can monitor the build here.

typescript-bot avatar Oct 27 '22 21:10 typescript-bot

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/137209/artifacts?artifactName=tgz&fileId=6D735CDF90B236C78EBAB4D1B24AB1290CFD58FF430870FD2F6749954247504902&fileName=/typescript-4.9.0-insiders.20221027.tgz"
    }
}

and then running npm install.

typescript-bot avatar Oct 27 '22 21:10 typescript-bot

Pack this failed to produce a playground for this due to a failure over here: https://github.com/microsoft/TypeScript-Make-Monaco-Builds/actions/runs/3341170061/jobs/5532095999

node_modules/@types/node/globals.d.ts(72,13): error TS2403: Subsequent variable declarations must have the same type. Variable 'AbortSignal' must be of type '{ new (): AbortSignal; prototype: AbortSignal; abort(reason?: any): AbortSignal; timeout(milliseconds: number): AbortSignal; }', but here has type '{ new (): AbortSignal; prototype: AbortSignal; timeout(milliseconds: number): AbortSignal; }'.

Is this the broken node that @sandersn was talking about?

jakebailey avatar Oct 28 '22 01:10 jakebailey

The following has a pretty opaque error message and I'm not sure why it's implemented incorrectly.

function fieldIdentity<T>(
    target: object,
    context: ClassFieldDecoratorContext<object, T>
): (initialValue: T) => T {
    return (initialValue: T) => initialValue;
}

function accessorIdentity<T>(
    target: object,
    context: ClassAccessorDecoratorContext<object, T>
): ClassAccessorDecoratorResult<object, T, T> {
    return {
        get() {
            return context.access.get.call(target);
        },
        set(value) {
            context.access.set.call(target, value);
        },
        init(value) {
            context.access.set.call(target, value);
            return value;
        },
    };
}

class A_ {
    // Errors - argument of type 'undefined' is not assignable to parameter of type 'object'.
    @fieldIdentity x = 1;

    // No errors
    @accessorIdentity accessor y = 2;
}

DanielRosenwasser avatar Oct 28 '22 20:10 DanielRosenwasser

The following has a pretty opaque error message and I'm not sure why it's implemented incorrectly.

function fieldIdentity<T>(
    target: object,
    context: ClassFieldDecoratorContext<object, T>
): (initialValue: T) => T {
    return (initialValue: T) => initialValue;
}

function accessorIdentity<T>(
    target: object,
    context: ClassAccessorDecoratorContext<object, T>
): ClassAccessorDecoratorResult<object, T, T> {
    return {
        get() {
            return context.access.get.call(target);
        },
        set(value) {
            context.access.set.call(target, value);
        },
        init(value) {
            context.access.set.call(target, value);
            return value;
        },
    };
}

class A_ {
    // Errors - argument of type 'undefined' is not assignable to parameter of type 'object'.
    @fieldIdentity x = 1;

    // No errors
    @accessorIdentity accessor y = 2;
}

target isn't the object, it's the thing being decorated. I will definitely need to work on the error messages.

Try this instead:

function fieldIdentity<T>(
    target: undefined, // fields get `undefined` for `target` since there's nothing
                       // installed on the object until initializers run
    context: ClassFieldDecoratorContext<object, T>
): (initialValue: T) => T {
    return (initialValue: T) => initialValue;
}

function accessorIdentity<T>(
    target: ClassAccessorDecoratorTarget<object, T>, // this is the `{ get, set }` you can replace.
    context: ClassAccessorDecoratorContext<object, T>
): ClassAccessorDecoratorResult<object, T, T> {
    return {
        get() {
            return target.get.call(this); // use target.get, not access.get
        },
        set(value) {
            target.set.call(this, value); // use target.set, not access.set
        },
        init(value) {
            // no need to call set, it will be set for you after `init` runs.
            // context.access.set.call(target, value);
            return value;
        },
    };
}

class A_ {
    @fieldIdentity x = 1; // ok
    @accessorIdentity accessor y = 2; // ok
}

For field decorators, the return value is a pipeline function that takes in the defined initializer, allowing you to mutate or replace the value. Conceptually, its somewhat like the following:

class C {
  @A @B x = 1;
}

// *very* rough approximation
var a = A();
var b = B();
class C {
  x = a(b(1));
}

For accessors, the init property of the return value functions similarly:

// *very* rough approximation
var a = A().init;
var b = B().init;
class C {
  x = a(b(1));
}

Also, using access in this way results in a stack overflow. Think of access like this:

// source
class C {
  @A accessor x = 1;
}

// *very* rough approximation
A(target, { access: { get() { return this.x; }, set(v) { this.x = v; } });
class C {
  accessor x;
}

If you replace the getter with one that does context.access.get.call(this), you essentially just recursively call the same getter until you run out of stack space.

rbuckton avatar Oct 29 '22 01:10 rbuckton

The following has a pretty opaque error message and I'm not sure why it's implemented incorrectly.

@DanielRosenwasser: I made a few changes in b5a2ad6 to ensure a more useful error is produced.

rbuckton avatar Nov 01 '22 16:11 rbuckton

@typescript-bot pack this

rbuckton avatar Nov 01 '22 16:11 rbuckton

Heya @rbuckton, I've started to run the tarball bundle task on this PR at c837d78d7ee5bab2ab6f2c3c7c24882ca9c31dcf. You can monitor the build here.

typescript-bot avatar Nov 01 '22 16:11 typescript-bot

Hey @rbuckton, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/137418/artifacts?artifactName=tgz&fileId=AB77B9F7C08B0A35E9947DAD772F3CFFC61A6E13DD551F32AB732DDCDD7DF4F102&fileName=/typescript-5.0.0-insiders.20221101.tgz"
    }
}

and then running npm install.

typescript-bot avatar Nov 01 '22 16:11 typescript-bot

Can you add a esDecorators-classExpression-namedEvaluation test for auto accessors?

{ class C { static accessor x = class { @dec y: any }; } }

Per https://arai-a.github.io/ecma262-compare/snapshot.html?pr=2417#sec-createfieldinitializerfunction, one would expect the anonymous class is named as x.

JLHwung avatar Nov 02 '22 21:11 JLHwung

Seems like there's a mistake in definitions, ClassFieldDecoratorFunction got ClassAccessorDecoratorContext as context typings:

// #region type ClassFieldDecoratorFunction
/**
 * Describes a function that can be used to decorate a class field.
 */
type ClassFieldDecoratorFunction = <
    This,
    In,
    Out = In,
    Value = Out
>(target: undefined, context: ClassAccessorDecoratorContext<This, Value>) => ((this: This, initialValue: In) => Out) | void;
// #endregion

linbudu599 avatar Nov 04 '22 09:11 linbudu599

@sandersn can you take one more look at the decorators.d.ts lib types? I added more documentation and cleaned up a few things.

rbuckton avatar Nov 04 '22 21:11 rbuckton

Hey @rbuckton , I found some problems while trying out the new version of the decorator, the field decorator can replace the initial value by returning a function that uses the initial value as an input and can replace it with the return value, however in current TS only works for fields that already have an initial value, but in Babel(@babel/plugin-proposal-decorators + "version": "2022-03") it is possible to replace the value of a field with the decorator regardless of whether it has an initial value or not, I didn't find any definite information about this in the decorator's proposal, so I want to make sure if this is a bug or expected behavior?

The minimial code to repro:

import assert from "assert";

/** @type {ClassFieldDecoratorFunction} */
function Double() {
  return (v) => v * 2;
}

/** @type {ClassFieldDecoratorFunction} */
function Init() {
  return (v) => 1000;
}

class Foo {
  @Double
  count = 599;

  @Init
  init;
}

assert.equal(new Foo().count, 1198);
// Error, expect undefined === 1000
assert.equal(new Foo().init, 1000);

linbudu599 avatar Nov 07 '22 08:11 linbudu599

Hey @rbuckton, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/137418/artifacts?artifactName=tgz&fileId=AB77B9F7C08B0A35E9947DAD772F3CFFC61A6E13DD551F32AB732DDCDD7DF4F102&fileName=/typescript-5.0.0-insiders.20221101.tgz"
    }
}

and then running npm install.

And, I'm using this version, if the problem has already got fixed in newer build, can you give me the latest link?

linbudu599 avatar Nov 07 '22 08:11 linbudu599

@typescript-bot pack this

(I can't answer your questions but I can rerun the task.)

jakebailey avatar Nov 07 '22 16:11 jakebailey

Heya @jakebailey, I've started to run the tarball bundle task on this PR at aece08d08de4a04a3c55726b9b9b8c9841dfa636. You can monitor the build here.

typescript-bot avatar Nov 07 '22 16:11 typescript-bot

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/137715/artifacts?artifactName=tgz&fileId=AB06AB86AC7A77E6A0417052987F3F7E949641A66A177125A4E091AD10B5E3AD02&fileName=/typescript-5.0.0-insiders.20221107.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

typescript-bot avatar Nov 07 '22 16:11 typescript-bot

Hey @rbuckton , I found some problems while trying out the new version of the decorator, the field decorator can replace the initial value by returning a function that uses the initial value as an input and can replace it with the return value, however in current TS only works for fields that already have an initial value, but in Babel(@babel/plugin-proposal-decorators + "version": "2022-03") it is possible to replace the value of a field with the decorator regardless of whether it has an initial value or not, I didn't find any definite information about this in the decorator's proposal, so I want to make sure if this is a bug or expected behavior?

This was a bug, and has been fixed in d65c50b.

rbuckton avatar Nov 08 '22 06:11 rbuckton

@typescript-bot pack this

rbuckton avatar Nov 08 '22 06:11 rbuckton