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

Remove Axios, replace with Fetch #863

Closed
lalalune opened this issue Apr 18, 2023 · 10 comments · Fixed by #973
Closed

Remove Axios, replace with Fetch #863

lalalune opened this issue Apr 18, 2023 · 10 comments · Fixed by #973

Comments

@lalalune
Copy link

Right now langchainjs is using a very old version of Axios. This requires us to npm install --legacy-peer-deps

Fetch is now supported in Node natively, which means the same interface will work on backend or frontend without the axios dependency. Since there are just a couple of uses of axios, and it will keep the project more terse, I would like to make a PR that removes the dependency on Axios and replaces it with native fetch. Can polyfill node-fetch for older clients as well.

If this is agreed to, I can make a PR and verify the tests.

@nfcampos
Copy link
Collaborator

We use this version because it's the same version used by openai sdk, see #816 (comment)

We do try to use fetch where we can, you'll see we make use of it in quite a few places.

@jpamorgan
Copy link

hmmm. this is making it very hard to use with nestjs which needs ^1.35

@nfcampos
Copy link
Collaborator

Well but it'd be exactly as hard to use the openai sdk with it, no? That sounds like an issue to pick up with nestjs or openai. If openai update their version of axios we'll happily update it here as well.

@mikkotikkanen
Copy link

mikkotikkanen commented Apr 24, 2023

I am using latest axios in the project I have openai installed and it works just fine. Unfortunately, for some reason, langchain runs into the Conflicting peer dependency error 🤔

Here is the relevant part from package.json:

{
  "dependencies": {
    "axios": "^1.3.4",
    "openai": "^3.2.1",
  }
}

@nfcampos
Copy link
Collaborator

(That would be because npm treats peer dependency (axios is an optional peer dependency of langchain) ranges more strictly than direct dependency ranges (axios is a direct dependency of openai))

In any case I've opened PR #973 relaxing the peer dependency range for axios to make you folks happy (but I still recommend you use the axios version your dependencies (like openai) require, even if npm doesn't complain)

@nfcampos nfcampos linked a pull request Apr 24, 2023 that will close this issue
@mikkotikkanen
Copy link

Uhh, thank you! ❤️

Hmm. Now that I look at this in more detail, what exactly is the issue? Both packages seem to set the version similarly but for some reason openai is happy with any version you might have?

image

image

@nfcampos
Copy link
Collaborator

That ^1.0.0 is our dev dependency range (which doesn't affect anyone installing langchain from npm/yarn/etc). Our peer dependency range is now *. As far as I can tell both versions are compatible enough that in ours/openai's usage they can be used interchangeably

@mikkotikkanen
Copy link

Ah, right, just noticed it was under devDependencies 🤦‍♂️

@tecoad
Copy link

tecoad commented Apr 26, 2023

hmmm. this is making it very hard to use with nestjs which needs ^1.35

Same issue here.
Did you manage to make it work with nestjs?

@nfcampos
Copy link
Collaborator

@tecoad the PR mentioned above was released in 0.0.64 id suggest updating

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

Successfully merging a pull request may close this issue.

5 participants