swagger-js

swagger-js copied to clipboard

node-fetch@3 is out for some it. Unfortunately it's pure ESM so we cannot use it. We need to convert this library to ESM to use it (https://github.com/swagger-api/swagger-js/issues/2414).

Any updates on plans here? Trying to use swagger-client in a corporate environment, I am currently receiving security vulnerability warnings triggered by the version of node-fetch being used (with a recommended resolution to use node-fetch 3.2.10...or stop using swagger-client)

@reinrl,

Sorry for late response. In your case the solution is simple. Use Node.js >= 18 and npm overrides that would look like this:

{
  "overrides": {
    "swagger-client": {
      "node-fetch": "^3"
    }
  }
}

npm will not even install node-fetch@2 and your code will use native Node.js >=18 fetch (undici implementation).

After some research I came into the following conclusion: If we want to maintain backward compatibility (which we do) we have following options

1. Use native Node.js >= 18 fetch implementation

Wait until Node.js >=12 <18 dies out and we'll claim that swagger-client requires Node.js >= 18. For older Node.js following polyfills can be used:

• for Node.js >= 12.20.0 <15 only node-fetch@3 polyfill can be used
• for Node.js >=14, undici or node-fetch@3 polyfills can be used

2. Use node-fetch@3 with import() function

Given our usage of fetch function, this is currently very possible as we only use fetch function within async functions only.

3. Use node-fetch-cjs@3

https://www.npmjs.com/package/node-fetch-cjs is a CommonJS compatible version of node-fetch@3.

---

Generally I want to avoid integrating node-fetch@3 just to be doing the same, some time later with native Node.js fetch (undici impl). Pragmatically it just make sense to drop support for Node.js 12, and claim that SwaggerClient now supports Node.js >= 14.0.0. Use undici in isomorphic way - which means that browser env will use native browser fetch impl, Node.js >14 <18 will use the undici package, and Node.js >=18 will use the native Node.js fetch implementation (which is undici under the hood). So the solution would be the variation of 1..

@jimmywarting any comments?

Here is PR with Node.js >= 14 support and integrating undici: https://github.com/swagger-api/swagger-js/pull/3137

I would now recommend using:

1. Built in fetch if available (would req NodeJS v18)
2. Fallback to using undici (would req NodeJS v16.8)
3. then lastly using node-fetch@3 (would req NodeJS ^12.20.0 || ^14.13.1 || >=16.0.0)

i would not recommend using the forked cjs version, using async import() works just fine from cjs, undici do actually lazy loaded fetch right in the source code.

// looks kind of like this in undici
module.exports.fetch = async function fetch (resource) {
    fetchImpl ??= require('./lib/fetch').fetch
    return await fetchImpl(...arguments)
}

it's no different from our recommendation in https://github.com/node-fetch/node-fetch/issues/1279#issuecomment-915060754

const fetch = (...args) => import('node-fetch').then(({default: fetch}) => fetch(...args))

That way you can still use node-fetch@3 from a cjs project while still loading esm-only modules.

node-fetch-cjs is not in sync with the main source.

---

It would also however be nice to not having to download any fetch dependency at all. if i'm using NodeJS v18 or v20 then i just feel that it's a bit in vain to have to download any extra dependency. maybe some other recommendation could be to just let the user polyfill fetch with whatever implementation they prefer. or make it a optional peer dependency where you try to load fetch from either

const fetch = globalThis.fetch || await import('undici')
  .then(mod => mod.fetch)
  .catch(err => import('node-fetch'))
  .then(mod => mod.fetch)
  .catch(err => {
    throw new Error('fetch is missing, this require nodejs v18 or you have to install node-fetch, undici or polyfill it your self')
  })

i would also say that it might just be the time to also drop cjs and only provide a esm-only i think dual package is a hazard

Hi @jimmywarting,

Thanks for input.

Built in fetch if available (would req NodeJS v18) Fallback to using undici (would req NodeJS v16.8) then lastly using node-fetch@3 (would req NodeJS ^12.20.0 || ^14.13.1 || >=16.0.0)

Yeah, that's what I ended up doing during this night. It's good that my line of thinking wasn't that off. The only issue is that I'm testing exclusively against undici npm package in tests (in future when minimal supported Node.js is >18, we'll test exclusively against native Node.js fetch (which is again undici impl))

i would not recommend using the forked cjs version, using async import() works just fine from cjs, undici do actually lazy loaded fetch right in the source code.

Tried (and will probably look into it again), but it blows up the Jest. It would also blow up any downstream project using swagger-client.

Following jest config needs to be introduced to make it work:

  transformIgnorePatterns: [
    '/node_modules/(?!(node-fetch|data-uri-to-buffer|fetch-blob|formdata-polyfill)/)',
  ],

For not I've settled with: https://www.npmjs.com/package/node-fetch-commonjs

It would also however be nice to not having to download any fetch dependency at all. if i'm using NodeJS v18 or v20 then i just feel that it's a bit in vain to have to download any extra dependency. maybe some other recommendation could be to just let the user polyfill fetch with whatever implementation they prefer. or make it a optional peer dependency where you try to load fetch from either

Well that would be ideal, but our first goal is backward compatibility and out of the box functionality. If minimum supported version is Node.js >18, we will be there automatically, but our minimum supported Node.js is currently 12.20.0.

One can also decide to choose it's own fetch implementation by doing the following before importing swagger-client:

globalThis.fetch = myFetchImpl;
globalThis.Headers = myHeadersImpl;
globalThis.Request = myRequestImpl;
globalThis.Response = myResponseImpl;

i would also say that it might just be the time to also drop cjs and only provide a esm-only I think dual package is a hazard

As said above, that would break backward compatibility. We cannot afford this now. What we'll be doing is to introduce backward compatible ESM and we'll keep it as long as possible (until one of the dep we use happens to become ESM and there will be no replacement).

Thanks again @jimmywarting

Closed by https://github.com/swagger-api/swagger-js/pull/3137

:tada: This issue has been resolved in version 3.21.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: