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

Decorate constructor arguments with access modifiers #495

Closed
cyberprodigy opened this issue Dec 16, 2019 · 6 comments
Closed

Decorate constructor arguments with access modifiers #495

cyberprodigy opened this issue Dec 16, 2019 · 6 comments
Assignees
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request Wontfix ❌ This will not be worked on

Comments

@cyberprodigy
Copy link

cyberprodigy commented Dec 16, 2019

Is your feature request related to a problem? Please describe.
We are using extensively cool TypeScript feature which allows parameters from constructor map to class members, Example:

export class User {
  constructor(
    public id: number,
    public username: string,
 ) { }
}

then

let admin = new User(1,'admin');
console.log(admin.id) // prints 1

Describe the solution you'd like
I would like to be able to decorate these parameters as well

Describe alternatives you've considered
Don't use this TS feature

@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request labels Dec 16, 2019
@MichalLytek
Copy link
Owner

For now, @Field() is a MethodAndPropDecorator, so you can't do that.

While it's possible to add support for parameters in constructor, it's an error prone pattern as you can easily missplace the parameter and swap mobilePhone with homePhone, age with balance and others.

If you really want to create a class instance, it's better to accept the anonymous object in constructor and call Object.assign inside of it.

@backbone87
Copy link

it's an error prone pattern

every pattern can be missused, but there are a lot of use cases where object types have very few arguments, or are constructed by a factory or builder pattern.

just to have an example for a valid use case:

@InterfaceType()
abstract class UpdateUserResult {
  public constructor(
    @Field(() => User)
    public readonly user: User,
  ) {}
}

@ObjectType({ implements: UpdateUserResult })
class UpdateUserResultOk extends UpdateUserResult {}

@ObjectType({ implements: UpdateUserResult })
class UpdateUserResultRejected extends UpdateUserResult {
  public constructor(
    user: User,
    @Field(() => String)
    public readonly reason: string,
  ) {
    super(user);
  }
}

@MichalLytek
Copy link
Owner

Should be fairly easy to implement, moving to 1.x release milestone 👀

@MichalLytek MichalLytek added this to Backlog in Board via automation May 17, 2020
@ruohki
Copy link

ruohki commented Aug 22, 2020

this feature is very welcome to easily form up responses in queries mutations etc

@MichalLytek MichalLytek moved this from Backlog to To Do in Board Aug 30, 2020
@ruohki
Copy link

ruohki commented Mar 14, 2021

Any progress on this? Quality of Life feature :)

@MichalLytek MichalLytek self-assigned this May 22, 2021
@MichalLytek MichalLytek modified the milestones: 2.x versions, 2.0 release Oct 26, 2023
@MichalLytek
Copy link
Owner

I've checked and unfortunately this is not possible 😞
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-0.html#more-accurate-type-checking-for-parameter-decorators-in-constructors-under---experimentaldecorators

When using decorator on constructor argument, it's treated like a function parameter decorator.
So we have no propertyKey information, only the parameter index.
So we can't make it working easily.

The only solution would be to duplicate the property name in the decorator options:

@ObjectType()
export class User {
  constructor(
    @Field(() => Int, { propertyName: "id" })
    public id: number,
    
    @Field({ propertyName: "username" })
    public username: string,
 ) { }
}

Which makes no sense, it's same duplication like now with defining constructor params apart from the fields definitions.

@MichalLytek MichalLytek closed this as not planned Won't fix, can't repro, duplicate, stale Jan 10, 2024
@MichalLytek MichalLytek added Wontfix ❌ This will not be worked on and removed Discussion 💬 Brainstorm about the idea labels Jan 10, 2024
@MichalLytek MichalLytek removed this from To Do in Board Jan 10, 2024
@MichalLytek MichalLytek removed this from the 2.0 release milestone Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request Wontfix ❌ This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants