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

feat: support validating parameter properties and TypeScript constructor shorthands #1669

Closed
205g0 opened this issue Jun 5, 2022 · 4 comments
Labels
status: wontfix type: feature Issues related to new features.

Comments

@205g0
Copy link

205g0 commented Jun 5, 2022

Resubmit of #350, cc @raffomania, @NoNameProvided

Not using constructor parameters makes generating new instances very cumbersome. Why cannot constructor parameters also be validated? I know it's a different implementation but still, it's an important use case.

@205g0 205g0 added flag: needs discussion Issues which needs discussion before implementation. type: feature Issues related to new features. labels Jun 5, 2022
@braaar
Copy link
Member

braaar commented Nov 14, 2022

Is it actually possible to do what @raffomania suggests in the last comment in #350? If the idea is to merely add support for a syntax convenience I don't see a problem with this change. However, I would still say it could be confusing to people who are not used to this syntax. It can easily start to look like your are validating the arguments of the constructor function before instantiation a class.

I'm not familiar with parameter decorators, so I'm not sure what to make of the typescript docs here.

@NoNameProvided
Copy link
Member

NoNameProvided commented Nov 16, 2022

Is it actually possible to do what @raffomania suggests in the last comment in #350?

Technically it is possible yes, we can have parameter decorators. However, those behave differently than property decorators which means we would need to update all validation decorators to handle both cases.

Not using constructor parameters makes generating new instances very cumbersome. Why cannot constructor parameters also be validated?

This is a lot of work for a small benefit. Comparing the cost of increased maintenance to the benefit of not needing to assign properties manually in the constructor is simply put not worth it.

There is also a huge difference when the decorators run: currently, after the class instance is created, in the proposed solution they run before the class instance is created. What should we do for example when some validation decorators are present only as property decorators, should we fail the class creation because we cannot access their property content yet, or should we ignore them? This is not a simple thing to implement and there are a lot of other use cases to think about. We don't want to implement features that are useful for a single specific use-case and break or not play nicely together with all the others. (Example: only some of the properties are in the constructor, the rest is computed from received properties.)

To be clear I think my initial suggestion still works, you don't need to manually assign properties when you are using definite assignment assertion (!) to tell Typescript it will be assigned later.

To summarize the situation, the initial problem is the following:

class User {
  @IsEmail()
  email: string;

  constructor(email: string) {
    this.email = email; // <- users don't want to specify this
  }
}

The above example showcases the problem of OP is that values must be assigned for each property in the constructor because of the --strictPropertyInitialization TypesScript option.

The solution is to use definite assignment assertion:

class User {
  @IsEmail()
  email!: string; // No explicit assignment needed, users can assign it later
}

Closing this as wontfix for now.

@NoNameProvided NoNameProvided added status: wontfix and removed flag: needs discussion Issues which needs discussion before implementation. labels Nov 16, 2022
@NoNameProvided NoNameProvided changed the title Support validating parameter properties/TS Constructor Shorthands feat: support validating parameter properties and TypeScript constructor shorthands Nov 16, 2022
@NoNameProvided
Copy link
Member

If you still feel the need for this, you can open a discussion as a feature idea. There the community can discuss and specify the requirements, API, etc. I am telling you in advance this is significant work to get it right and the final proposed solution may not be accepted for implementation.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: wontfix type: feature Issues related to new features.
Development

No branches or pull requests

3 participants