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

Browser Support for JS SDK #18

Closed
osdevisnot opened this issue Feb 13, 2024 · 6 comments
Closed

Browser Support for JS SDK #18

osdevisnot opened this issue Feb 13, 2024 · 6 comments
Assignees

Comments

@osdevisnot
Copy link
Contributor

osdevisnot commented Feb 13, 2024

I am attempting to use the SDK client in browser environment. However, this results in below errors:

image

Can we please author this SDK in a manner that is compatible with both node.js and browser?

If the is willing to support this feature, I can even attempt to submit a PR to that effect.

@dmoss18
Copy link
Contributor

dmoss18 commented Feb 13, 2024

Yes we should be supporting both. I'll take a look at this. Thank you for bringing it up.

@dmoss18 dmoss18 self-assigned this Feb 13, 2024
@osdevisnot
Copy link
Contributor Author

Thank you @dmoss18. Looks like this was introduced in f14d2e5 (#6). This PR tried to enforce the minimum version for TLS to 1.2, however our node support target is >=20.0.0, where the default version is already 1.2 as documented in the node docs.

@dmoss18
Copy link
Contributor

dmoss18 commented Feb 13, 2024

@osdevisnot I have a branch up here. Try it and lmk if it helps.

@osdevisnot
Copy link
Contributor Author

It helps; but I think we might need to find a better solution to browser vs node compatibility scene. For eg: the try catch approach in #24 still results in warnings as noted #24 (review); mainly because we still import https and attempt to use https.Agent which is not supported in browser environments. I also created a separate issue #25 (comment) which should really be part of this one.

@dmoss18
Copy link
Contributor

dmoss18 commented Feb 28, 2024

@osdevisnot I have merged PRs for the issues you mentioned. Please confirm that this issue is resolved and I will close and publish a new version of the package.

@dmoss18 dmoss18 closed this as completed Feb 28, 2024
@dmoss18 dmoss18 reopened this Feb 28, 2024
@osdevisnot
Copy link
Contributor Author

This issue is fixed. However I forgot to add one change which should really be part of #19. I will create a separate PR for that soon.

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

No branches or pull requests

2 participants