sfcc-ci icon indicating copy to clipboard operation
sfcc-ci copied to clipboard

Replace deprecated request package with alternative

Open tobiaslohr opened this issue 5 years ago • 6 comments

NPM package request has been deprecated. We should work to look out for suitable alternatives and start exploring them. One candidate might be got (https://www.npmjs.com/package/got), however alternative options are welcome.

tobiaslohr avatar Apr 07 '20 14:04 tobiaslohr

got (like all of Sindre's packages nowadays) is ESM only in the current version. Given that sfcc-ci is commonjs at the moment that adds some friction since you can't do a simple require or you need to use a version < than the current.

I find axios (https://www.npmjs.com/package/axios) works fine for OCAPI, webdav (i.e. for both text and binary content). Still works fine with commonjs and is promise-native. So if we ever want to drop support for node<12 (which itself is EOL in April anyway) we could use it async as well. I use it here for ocapi/webdav and AM: https://github.com/SalesforceCommerceCloud/b2c-tools/blob/main/lib/environment.js

clavery avatar Feb 10 '22 15:02 clavery

axios seems like a more straightforward replacement since it keeps the same architecture patterns. While I like the fact that sfcc-ci follows similar design patterns as the storefront architecture, with pwa-kit this is less important. Since sfcc-ci is used by many different environment combinations I would consider node version compatibility important. Either got or axios would work since they seem to be the most well maintained, just have to understand the trade-offs.

nickpalmigiano avatar Feb 10 '22 20:02 nickpalmigiano

https://npmtrends.com/axios-vs-fetch-vs-got-vs-ky-vs-request-vs-simple-get-vs-superagent

Looks like Axios is very reliable alternative.

Currently, there're 3 warnings, and they're all related to request package:

npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
npm WARN deprecated [email protected]: this library is no longer supported
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142

@tobiaslohr do you need any support on this?

gokaygurcan avatar Apr 27 '23 11:04 gokaygurcan

@gokaygurcan Yes, I do! Happy to get support for this. I think there are a few candidates, also see this thread https://github.com/request/request/issues/3143.

While there are personal preferences I think we should ensure that the replacement accounts at least for the following principles:

  • Active community
  • Promise support (to be able to gracefully migrate from callbacks to promises)
  • Streaming support (for WebDAV requests)
  • Lightweight (arguable)
  • Usage in other tools (arguable)

Thoughts?

tobiaslohr avatar Apr 27 '23 11:04 tobiaslohr