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

feat(api): export SDK #856

Merged
merged 3 commits into from
Feb 14, 2024
Merged

feat(api): export SDK #856

merged 3 commits into from
Feb 14, 2024

Conversation

yoitsro
Copy link
Contributor

@yoitsro yoitsro commented Feb 13, 2024

🚥 Resolves #821

🧰 Changes

In some use cases, the SDK class needs to be instantiated several times.

For instance, in a multi-tenancy scenario, there may be multiple instances of the same API server, but serving different tenants' data from different base URLs. In this case, we need to instantiate multiple clients with different server URLs.

🧬 QA & Testing

The unit test fixtures have been updated accordingly to reflect that the SDK is now exported.

In some use cases, the SDK class needs to be instantiated several
times.

For instance, in a multi-tenancy scenario, there may be multiple
instances of the same API server, but serving different data from
different base URLs. In this case, we need to instantiate multiple
clients with different server URLs.
Copy link
Member

@kanadgupta kanadgupta left a comment

Choose a reason for hiding this comment

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

hi, thanks so much for this contribution! unfortunately when compiling TS into CJS and ESM, files with both default and non-default exports typically lead to typing issues in certain environments. we don't have great test coverage on this front quite yet unfortunately.

what if the class was the default export from a new src/sdk.ts file and the IIFE in src/index.ts imports that?1

Footnotes

  1. first discussed here

@yoitsro
Copy link
Contributor Author

yoitsro commented Feb 13, 2024

Yeah, I saw that discussion after submitting the PR but didn't have time to sort it. Let me take a quick look at it.

@yoitsro
Copy link
Contributor Author

yoitsro commented Feb 14, 2024

Here are the changes. Once they're good, I'll squash the commits down (or I guess you could do that too if required).

@kanadgupta kanadgupta added enhancement New feature or request area:api Issues related to the `api` CLI, which builds the SDKs labels Feb 14, 2024
Copy link
Member

@kanadgupta kanadgupta left a comment

Choose a reason for hiding this comment

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

thanks again, this looks great!

@kanadgupta
Copy link
Member

I'll squash the commits down (or I guess you could do that too if required).

and we'll take care of squashing when we merge, thanks!

Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

thanks for this!

@kanadgupta kanadgupta merged commit 4bfdb53 into readmeio:main Feb 14, 2024
5 checks passed
@yoitsro yoitsro deleted the feat/export-sdk branch February 14, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Issues related to the `api` CLI, which builds the SDKs enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

define the Type of the sdk object.
3 participants