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

Instance ID is required to be a number by types but the docs says it should be string #181

Open
isoroka-plana opened this issue Oct 7, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@isoroka-plana
Copy link

Describe the issue:
When using the getInstance() method in TypeScript and passing a string id as the first parameter, TypeScript complains that string is not assignable to a number because the first parameter of getInstance() is typed to be a number contrary to what the documentation says.

How to reproduce:
Install the lib into a TypeScript project and try calling AsgardeoSPAClient.getInstance('string')

Expected behavior:
There should be no TypeScript error. string is the preferred type for id.

Environment information (Please complete the following information; remove any unnecessary fields) :

  • SDK Version: 3.1.2

@isoroka-plana isoroka-plana added the bug Something isn't working label Oct 7, 2024
@brionmario
Copy link
Member

Hi @isoroka-plana,

Thanks for reporting the issue!

@DonOmalVindula What was our initial plan for typing this argument?
AFAIR, we set 0 as the default instance id.

Also, i don't see a particular reason for not supporting both string | number.

WDYT?

@DonOmalVindula
Copy link
Contributor

DonOmalVindula commented Oct 10, 2024

Hi @isoroka-plana,

Thanks for reporting the issue!

@DonOmalVindula What was our initial plan for typing this argument? AFAIR, we set 0 as the default instance id.

Also, i don't see a particular reason for not supporting both string | number.

WDYT?

Hi @brionmario and @isoroka-plana,

We have made instance ID to be a number as we have an internal logic to keep the count of instances created. But of course, we can make an improvement to accept a string by separating out the counting logic when a custom instance ID is provided. We will use this issue to track this as an improvement.

As a temporary measure, we will update the documentation to match the argument type (numbers) of the current implementation to avoid any confusion.

@isoroka-plana
Copy link
Author

@DonOmalVindula does it mean that the string id shouldn't be working right now if I suppress the type checker? Our use case is that we have multiple instances of the identity provider for each client (our app is meant to be integrated into apps of our clients) and we wanted to use the id of the client as id for the instance to make sure that if the user uses our app with both clients the app would still behave as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants