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: Allow specifying callback arity #1187

Merged
merged 2 commits into from
Jun 13, 2023
Merged

Conversation

xenia-lang
Copy link
Contributor

@xenia-lang xenia-lang commented Feb 20, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

The Callback nest passes to passport always has an arity of 0. The user can not influence this behavior.

Issue Number: 446

What is the new behavior?

When creating a new strategy, the user can ask nest to calculate the arity of the callback based on the arity of the validate function or specify an arity to use.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I am a little lost on where to put a test for this behavior as well as where to make changes for the documentation. If someone can point me in the right direction, I will gladly make the necessary changes.

I am aware that the issue is long closed and a workaround is suggested. However, I feel that needing that workaround is a shortcoming of nest. I also fell that the discussion on the issue does not consider all available options (e.g. allowing the user to decide).

I would suggest making the the calculation of the callback arity the default and allowing the user to deactivate it, if necessary. However, this would make it a breaking change, so for the time being, the old behavior is the default.

@xenia-lang
Copy link
Contributor Author

Is there any way I can further assist in the review of this PR?

@xenia-lang
Copy link
Contributor Author

@micalevisk Maybe?

@kamilmysliwiec kamilmysliwiec changed the base branch from master to 10.0.0 June 13, 2023 07:35
@kamilmysliwiec kamilmysliwiec merged commit 43db968 into nestjs:10.0.0 Jun 13, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants