search

tastytrade / tastytrade-api-js

Javascript sdk for the tastytrade api
MIT License
67 stars 36 forks source link

Browser Support for JS SDK #18

Closed osdevisnot closed 7 months ago

osdevisnot commented 7 months ago

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 commented 7 months ago

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

osdevisnot commented 7 months ago

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 commented 7 months ago

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

osdevisnot commented 7 months ago

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 https://github.com/tastytrade/tastytrade-api-js/pull/24 still results in warnings as noted https://github.com/tastytrade/tastytrade-api-js/pull/24#pullrequestreview-1879040448; 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 https://github.com/tastytrade/tastytrade-api-js/issues/25#issue-2133217745 which should really be part of this one.

dmoss18 commented 7 months ago

@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.

osdevisnot commented 7 months ago

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.