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

Export OptimizelyReactSDKClient as interface or type #49

Closed
kylemh opened this issue May 5, 2020 · 5 comments
Closed

Export OptimizelyReactSDKClient as interface or type #49

kylemh opened this issue May 5, 2020 · 5 comments

Comments

@kylemh
Copy link

kylemh commented May 5, 2020

The return value of createInstance (OptimizelyReactSDKClient) should be exported as a type, lest we need to use ReturnType<typeof createInstance>

@avifoxi
Copy link

avifoxi commented Aug 3, 2020

@kylemh it is ->

import { ReactSDKClient } from '@optimizely/react-sdk'

https://github.com/optimizely/react-sdk/blob/master/src/index.ts#L26

@kylemh
Copy link
Author

kylemh commented Aug 3, 2020

@avifoxi

No, I mean OptimizelyReactSDKClient which extends off of ReactSDKClient and is the explicit return type for createInstance:

export function createInstance(config: optimizely.Config): OptimizelyReactSDKClient {

@matthewbordas
Copy link

I agree. I ran into this issue this week. It would be really helpful to have this updated!

zashraf1985 pushed a commit that referenced this issue Feb 4, 2022
…tead of implementation class (#148)

### Summary
Return type of createInstance was OptimizelyReactSDKClient which is the implementation class. Changed it to the ReactSDKClient interface instead.

### Test Plan

- Manually tested thoroughly.
- All existing unit tests pass.

### Issues
#49
@zashraf1985
Copy link
Contributor

@kylemh @matthewbordas !
OptimizelyReactSDKClient is the implementation class which shouldn't have been exposed in the first place. We have changed the return type of createInstance to ReactSDKClient, which is already exported by the SDK. Let us know if it resolves your issue?

@kylemh
Copy link
Author

kylemh commented Feb 4, 2022

I am no longer at the company that had the codebase with this issue, so I won't be able to confirm for you, but I bet it works great!

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 a pull request may close this issue.

4 participants