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

[Fix] Change type of unused req param to unknown to resolve type conflicts #394

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zackdotcomputer
Copy link

@zackdotcomputer zackdotcomputer commented Jan 4, 2024

Description

This PR resolves #393 by relaxing the type requirements around the req parameter in the callbacks returned by both the expressJwtSecret and passportJwtSecret integrations. This parameter is not used by the implementations of either function, so it can be safely relaxed to an unknown type. This would allow any value to be inputted, maintaining both backwards and forwards compatibility with type changes in the related express and passport libraries.

The alternative proposed in the issue would be to resolve this new conflict by updating the type to stay precisely in step with passport, but that would introduce unnecessary backwards-compatibility problems for people who were on earlier versions of @types/passport. Since this type's only purpose is to maintain conformance with the external library's types, going with the most permissive solution seems best.

References

Testing

This change is only to make the typescript typing more permissive, and so I believe no change to the testing plan is required.

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not the default branch (I think so, I'm using the default branch?)

@zackdotcomputer zackdotcomputer requested a review from a team as a code owner January 4, 2024 12:18
@zackdotcomputer zackdotcomputer changed the title [Fix] Change type of unused req param to unknown to avoid needless conflicts [Fix] Change type of unused req param to unknown to resolve type conflicts Jan 4, 2024
@icco
Copy link

icco commented Jan 25, 2024

@evansims could we please get this merged?

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.

Can't match types definition in @types/passport-jwt@4.0.0
2 participants