fix: corrected optional parameter type #144
Conversation
|
Realised these are auto generated so I've had to publish a temp package for now so we can fix our issues |
|
Thanks @bashleigh, I agree with you that actually we'll need an override declaration here instead of If you've already patched it in such a way (maybe in a fork), are you able to update the PR with that change? I think the declarations would be something like: verify(token: string, requestedNonceOrCb: string, cb: verifyCallback): any;
verify(token: string, cb: verifyCallback): any; |
|
I can however when publishing with microbundle? The types are updated and these changes removed |
|
Gotcha. It looks like we should be able to fix this just with a JSDocs modification but there are (at least) a couple of open bugs that prevent this from working:
I think we're either going to have to turn off types generation (for now) and fix it manually until either type generation can work with this, or we change the API so that we don't need an overload. A workaround in the meantime would be to just pass Happy to use the workaround for now, and for me to close this while we work out our long-term strategy here? |
|
Thanks for your reply, sure! I am able to pass undefined however the error with the types I wasn't able to ignore (I think it's down to yarn workspaces + webpack but not entirely sure) which affected my build process. For now I can use the alternative lib. If you need help building a DT lib or other method let me know and I'd be happy to help out! |
Changes
Please describe both what is changing and why this is important. Include:
Summary
In the types for the verify method there is an optional param
requestedNoncewith no default value which is followed by a required parametercb. This throws an error with tscTS1016and is preventing my team from fixing an issue with an alternative package (we decided to use this one to replace it).To fix this issue I've changed the
requestedNonceparameter to be of typestring | undefinedand made it a required parameter. However I'm not entirely sure if this is the correct solution and perhaps (after look at your codebase) you're looking for requestedNonce or callback which would require 2 different declarations shown below.Alternative declarations
Based on the codebase I think this is possibly what you're after
The above then allows requestedNonceOrCb or be either string or verifyCallback and depending on those types (if string) then the cb param is required else is not
References
Please include relevant links supporting this change such as a:
Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.
Checklist