Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorator spec normative changes - static accessors and natural this in decorators #53752

Closed
4 tasks done
Jamesernator opened this issue Apr 12, 2023 · 1 comment Β· Fixed by #55276
Closed
4 tasks done
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.

Comments

@Jamesernator
Copy link

Jamesernator commented Apr 12, 2023

Suggestion

πŸ” Search Terms

decorators, decorators normative changes

βœ… Viability Checklist

My suggestion meets these guidelines:

  • [?] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

This does change behaviour, but given this is a spec change TypeScript should be doing so regardless.

⭐ Suggestion

This is a spec change in the decorators proposal. TypeScript should implement these changes.

Changes

Of the changes, only static accessors and natural this in decorators are behaviour that will have to change. The other changes were either not implemented by TypeScript (SetFunctionName), spec bugs (HomeObject stuff), or the new behaviour is already implemented by TypeScript (addInitializer throws on non-function).

For natural this in decorators the following should not type error, and in down-leveling should not cause the assertion to fail either (playground link):

function assert(cond: boolean): asserts cond {
    if (!cond) {
        throw new Error(`assertion failed`);
    }
}

class DecoratorProvider {
    #x = 3;

    decorate<T>(this: this, v: T, ctx: DecoratorContext): T {
        // This assertion should not fail in down-leveled code
        assert(this !== undefined);
        return v;
    }
}

const instance = new DecoratorProvider();

class Foo {
    // Should not be a type error
    @instance.decorate
    method() {

    }
}

For static accessors this should succeed at runtime in down-leveling, the typing is already correct (playground link):

class A {
    static accessor x = 3;
}

class B extends A {}

// Currently throws when down-leveled, but should succeed
console.log(B.x);
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Apr 12, 2023
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.2.0 milestone Apr 12, 2023
@nicolo-ribaudo
Copy link

nicolo-ribaudo commented Apr 17, 2023

Just in case it's useful, a test case I found (thanks to TS type errors!) while implementing this in Babel:

declare var B: any;
declare var dec: any;

class A extends B {
    m() {
        class C {
            @(super.dec)
            m2() {}
        }
    }
}

where super must be rewritten to this wherever it's stored as the "this value".

@typescript-bot typescript-bot added Fix Available A PR has been opened for this issue labels Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants