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

Support object parameter and fix optionals #102

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

Conversation

alex-fusionauth
Copy link

I had to create new PR to update the old #37 to the latest codebase sorry @ianwensink.

I think this makes sense as most of the time in JS you will pass objects on the create.

ianwensink and others added 2 commits September 2, 2020 09:09
Both the constructor of FusionAuthClient and its exchangeOAuthCodeForAccessToken now also support calling them with an object instead of separate parameters. This helps when a lot of the parameters are optional (as per the docs). Completely backwards compatible.

#29 & #36
Copy link
Contributor

@lyleschemmerling lyleschemmerling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an interesting idea to solve the problem that the typescript typings are not correct, however, there's more that we will need to take into account.

The file FusionAuthClient.ts is generated. The source for that is here. Any changes made to this file directly will be over-written the next time we run the generator.

But that also means we do have the opportunity to address all of the methods in the client at once. We do need to maintain backwards compatibility, which this does, and we want to ensure that the pattern is consistent and sensible for the whole of the client. So if we want to make this change we will need to make it in the builder file and then ensure that it makes sense once it is built.

One issue we will want to consider when addressing this is that the JSON files which are the source for the client builder have "(Optional)" in the comments for the parameters but there isn't an explicit property declaring the parameter as optional, we should look into that because we should get the typings to indicate where they are optional correctly regardless of if we are passing an object or positional parameters

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.

4 participants