Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Replace request to node-fetch, #1076 #1125

Closed
wants to merge 1 commit into from

Conversation

Dima-Dim
Copy link

@Dima-Dim Dima-Dim commented Sep 23, 2023

  • All tests pass
  • I have run npm run doc

About tests:
I don't have TEST_STICKER_SET_NAME and TEST_PROVIDER_TOKEN to run the tests related to them. The rest of the test pass normally.

Description

Getting rid of deprecated dependencies request.

References

https://www.npmjs.com/package/request

Issue: #1076.

@kamikazechaser
Copy link
Collaborator

Looks like a good change if all tests pass, though Node.js has a built in fetch API in newer version (18+).

@Dima-Dim
Copy link
Author

Looks like a good change if all tests pass, though Node.js has a built in fetch API in newer version (18+).

Yes, after some time it will be possible to try to completely switch to a native solution, but i think no before end of support version older than 18.

@0x114514BB
Copy link

Your change literally removed the request dependency. The core _request function of NTBA depends on request-promise (also deprecated), which has request as a peer-dependency. So you should replace it with node-fetch or other solutions.

@Dima-Dim
Copy link
Author

Your change literally removed the request dependency. The core _request function of NTBA depends on request-promise (also deprecated), which has request as a peer-dependency. So you should replace it with node-fetch or other solutions.

Yes, this PR is the first step. The next one will require more changes and more time.

@danielperez9430
Copy link
Collaborator

For the moment we have already a remplace for the request library merge and without need to drop support for the current old versions.

In a future completed refactor, we will be drop the support for older versions of node. But for the moment we still preserve the compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants