getindiekit / mf2tojf2

Convert MF2 to JF2.
MIT License
6 stars 2 forks source link

Use builtin fetch if detected #19

Closed retronav closed 2 years ago

retronav commented 2 years ago

This library becomes unusable on Deno because of the usage of undici, which is solely a Node.js targeted library. This can also happen if this module is to be used in browsers. So, to address that, I added a ponyfill which will use native Fetch API if its present or else fall back to undici's Fetch API implementation.

Fetch API's coverage is huge, and using default Fetch would be make sense once v16 reaches EOL (2023-09-11), since v18 supports native Fetch.

Consider this as an effort to make this library more portable, and no hard feelings if this doesn't go through. This patch works for my use case.

paulrobertlloyd commented 2 years ago

@retronav Thanks for this PR, and sorry it’s taken me a while to get to reviewing it. One of the reasons I used Undici is because it’s the package that supports Node’s ‘native’ fetch under the hood, so I should be able to remove this dependency once this package requires Node v18.

I wasn’t aware of the Deno or browser compatibility issues however, and this seems like a fine way of providing support for those platforms while this package is still pegged to v16. I was half tempted to bump the minimum supported Node version instead, but that would require a version bump for this package, and I’m hesitant to do that just yet.

TL;DR: This is great, thank-you!