search

oracle / oci-typescript-sdk

Oracle Cloud Infrastructure SDK for TypeScript and JavaScript
https://docs.oracle.com/en-us/iaas/Content/API/SDKDocs/typescriptsdk.htm
Other
68 stars 50 forks source link

Use of isomorphic-fetch mutates global `fetch` unnecessarily, causing problems with other libraries #259

Open wilsonzlin opened 6 months ago

wilsonzlin commented 6 months ago

The isomorphic-fetch dependency mutates the global fetch value. This causes problems with some other libraries that also do this in some other way. For example, all fetch calls from this SDK failed when I loaded the cohere-ai library, which also mutates global.fetch (I have raised an issue there too).

It appears that this isn't necessary, as the fetch call is only used in 2 places (excluding examples) across the entire codebase: circuit-breaker.ts and http.ts. These can be trivially migrated to using an imported fetch (see below) that avoids mutating the global value, which is actually already done in url-based-x509-certificate-supplier.ts.

Also, the dependence on both isomorphic-fetch and node-fetch appears to be redundant (the former loads the latter on Node.js). I would suggest replacing both with fetch-ponyfill instead, which is recommended by the authors of isomorphic-fetch; it doesn't mutate the builtin, and provides both whatwg-fetch for browsers and node-fetch for Node.js. (It appears that node-fetch is also only used in 1 place.)

jyotisaini commented 6 months ago

@JoshuaWR : Can you take a look ? I think we should be able to replace global fetch with the imported fetch.

JoshuaWR commented 6 months ago

I can take a look. Thank you @wilsonzlin for bringing this to our attention, I will look into it and get back to you.

JoshuaWR commented 6 months ago

Hi @wilsonzlin, just want to clarify on something. We might not want to migrate to fetch-ponyfill, and instead continue to use isomorphic-fetch. If we include 'import 'isomorphic-fetch' in each file where we call fetch, will this solve the issue you are facing? Or will the inclusion of isomorphic-fetch at all lead to mutation of global fetch?

wilsonzlin commented 6 months ago

I believe the import of isomorphic-fetch itself will mutate the global fetch regardless of where or how many times it's imported, and therefore continue this problem according to the docs.

I also found cross-fetch which is more widely used and supported and may be an alternative to fetch-ponyfill.