oracle/oci-typescript-sdk

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

Opened this issue · 4 comments

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

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

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

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?

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.