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

Decorators not allowed classes expressions #7342

Open
DavidKDeutsch opened this issue Mar 2, 2016 · 31 comments
Open

Decorators not allowed classes expressions #7342

DavidKDeutsch opened this issue Mar 2, 2016 · 31 comments
Labels
Domain: Decorators The issue relates to the decorator syntax Suggestion An idea for TypeScript Waiting for TC39 Unactionable until TC39 reaches some conclusion

Comments

@DavidKDeutsch
Copy link

Not sure if this is by design or not, but the following gives a compile error of "Decorators are not valid here" with TypeScript 1.8:

let testClass = new class {
    testMethod(@myDecorator date: Date): any {
        return date;
    }
}();
@mhegazy
Copy link
Contributor

mhegazy commented Mar 2, 2016

they were never enabled for class expressions. so the behavior has not changed. but they should.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Mar 2, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Mar 2, 2016
@mhegazy mhegazy added the Committed The team has roadmapped this issue label Mar 2, 2016
@mhegazy mhegazy changed the title Parameter decorators not allowed on methods of anonymous classes Decorators not allowed classes expressions Apr 28, 2016
@mhegazy mhegazy modified the milestones: Future, TypeScript 2.1 Sep 21, 2016
@mhegazy mhegazy added the Domain: Decorators The issue relates to the decorator syntax label Sep 21, 2016
@dcworldwide
Copy link

dcworldwide commented Feb 23, 2017

This also applies when using the new 2.2 mixin design.

I.e.


/** Any type that can construct *something*. */
type Constructor<T> = new (...args: any[]) => T;

function ViewModel<T extends Constructor<MyModel>>(Base: T) {
	return class extends Base {

                // Won't compile. Decorator not allowed here
		@observable
		IsReadOnly: boolean;

                // Won't compile. Decorator not allowed here
		@computed get SomeProp(): string[] {
			return []
		}

		constructor(...args: any[]) {
			super(...args);
			this.IsReadOnly = true
		}
	}
}

Is this something that should work?

@selsamman
Copy link

selsamman commented Mar 10, 2017

Not being able to use decorators with the mixin pattern for us means not being able to use the new mixin pattern at all. Is there a new milestone for this?

@43081j
Copy link

43081j commented Apr 27, 2017

any movement on this? we're still unable to use the new mixin pattern because of this.

it seems fixable by naming the class but should probably work on anonymous classes too...

@selsamman
Copy link

If you follow the link above #14607, it seems there is a simple work around. The whole problem seems to be that you can't return directly a class with decorators but if you define the class with a local name and then return it everything is good. We have been using that pattern and taking advantage of mixins with decorators.

@vojtechhabarta
Copy link

Here is an example how to use decorators on mixins properties: http://stackoverflow.com/questions/41318403/implement-pure-class-mixins-with-typescript/43162485#43162485

@apexskier
Copy link

Just figured out this workaround as well. Here's a simple repro showing where it works and doesn't.

function testDecorator(): PropertyDecorator {
    return function internal() {
        console.log("testing");
    };
}

class Works {
    @testDecorator()
    method(): string {
        return "works";
    }
}

function makeClassWorks() {
    class Generated {
        @testDecorator()
        method(): string {
            return "works";
        }
    }
    return Generated;
}

function makeClassDoesntWork() {
    return class {
        @testDecorator()
        method(): string {
            return "doesn't work";
        }
    };
}
tsc --experimentalDecorators ./test.ts 
narwhal/src/test.ts(26,9): error TS1206: Decorators are not valid here.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 5, 2017

I just ran into this. I see it is a future milestone. Any chance it can get scheduled?

@rjamesnw
Copy link

I just ran into this also.

@DanielRosenwasser
Copy link
Member

My prediction is that we'll tackle this once decorators reach stage 3.

@szymski
Copy link

szymski commented Dec 5, 2019

It still doesn't work. Is it ever going to be fixed?

@Ackos95
Copy link

Ackos95 commented Mar 23, 2020

This issue is still active, but with one update, suggested workaround doesn't work anymore. If I do something like

export const testF = () =>
  class Class {
    @Decorator()
    public property: string;
  };

It would complain about decorators with: Decorators are not valid here., while if I try to bypass it as suggested with:

export const testF = () => {
  class Class {
    @Decorator()
    property: string;
  }
  return Class;
};

It would complain about using private property: Exported variable 'testF' has or is using private name 'ButtonStructureItemData'. ts(4025).

Any suggestions or workarounds?

@daweedm
Copy link

daweedm commented May 5, 2020

For now, ugly but working, just call your decorators as functions :

class Some {
    @decorate()
    prop: string;
}

becomes

const Some = class {
    prop: string;
}

decorate()(Some.prototype, 'prop');

// (...)

return Some;

@Ackos95
Copy link

Ackos95 commented May 6, 2020

@daweedm wouldn't extracting const Some = class {} and then returning that Some again cause same issue as declaring class and returning it back? And if not, why it wouldn't, what's the difference?

@TheMrZZ
Copy link

TheMrZZ commented May 15, 2020

I just had the same issue. For documentation purposes, I'll add that this bug concerns the class fields syntax too.

Example:

class RootClass {
  nestedClass = class {
    @decorator
    test() {}
  }
}

Will result in

TS1206: Decorators are not valid here.

@ericmorand
Copy link

Is there news on that issue? It's kinda easy to work around but it's still ugly:

const myFunctionThatReturnsAClass = () => {
    class ToReturn {
        @MyDecorator()
        run() {}
    }

    return ToReturn;
}

@canonic-epicure
Copy link

@ericanderson Try to compile the workaround code with declarations emit enabled, you'll receive an error.

Any chance this issue can be resolved?

@canonic-epicure
Copy link

There is PR for this feature, which passes all CI checks.

@DanielRosenwasser
Copy link
Member

We discussed this again at #44555

The conclusion we reached in the design meeting is that the risk, work, and maintenance doesn't outweigh the benefits, especially given the workarounds available today. On the bright side, I've seen more progress in standardizing decorators over the last year, so it's hopefully not too far off.

@brodo
Copy link

brodo commented Jun 14, 2021

@DanielRosenwasser is there a workaround that does not cause the TS4060 error? As @canonic-epicure said, the ones posted here don't really address the issue.

@RyanCavanaugh RyanCavanaugh added Waiting for TC39 Unactionable until TC39 reaches some conclusion and removed Committed The team has roadmapped this issue labels Jun 14, 2021
@RyanCavanaugh RyanCavanaugh removed this from the Backlog milestone Jun 14, 2021
@canonic-epicure
Copy link

@brodo There's no workaround that works with declaration files generation, afaik. There is a "green" PR, that passes all tests and fixes this problem: #42198

Unfortunately, TS team decided that it should not be merged.

You can try using a fork of TypeScript with this PR merged like this:

  "devDependencies": {
    "typescript": "git://github.com/bryntum/TypeScript.git#feat/7342"
  },

@RyanCavanaugh
Copy link
Member

I can't offer a workaround since this is "out of spec" territory. Decorators are not supported on class expressions. We're going to wait until TC39 standardizes the decorator behavior.

@ericmorand
Copy link

ericmorand commented Jun 15, 2021

@RyanCavanaugh, out of curiosity, since there is no standard, what spec did you use as a reference for the support of decorators by TypeScript?

@RyanCavanaugh
Copy link
Member

what spec did you use as a reference for the support of decorators by TypeScript?

The prior decorators proposal that was being worked on by TC39 around 2014, but was then abandoned in favor of a new decorators proposal which is now also at stage 2.

See also https://github.com/tc39/proposal-decorators#how-does-this-proposal-compare-to-other-versions-of-decorators

@a-tarasyuk
Copy link
Contributor

The decorators proposal has been moved to Stage 3.

@MichelJonkman
Copy link

This still doesn't seem to work in Typescript 5.3, would be a very nice feature.

@szymski
Copy link

szymski commented Dec 29, 2023

Any particular obstacles which stop this from being implemented?

@blipk
Copy link

blipk commented Aug 19, 2024

If anyone else comes across this I managed to solve my issue by setting the following in tsconfig.json
"experimentalDecorators": true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Decorators The issue relates to the decorator syntax Suggestion An idea for TypeScript Waiting for TC39 Unactionable until TC39 reaches some conclusion
Projects
None yet
Development

Successfully merging a pull request may close this issue.