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

Does fetch dependencies need to be updated? #11927

Closed
harryadel opened this issue Feb 16, 2022 · 7 comments
Closed

Does fetch dependencies need to be updated? #11927

harryadel opened this issue Feb 16, 2022 · 7 comments
Labels
good first issue Good first issue or something that should is nice to do. in-discussion We are still discussing how to solve or implement it Project:Fetch Fetch package

Comments

@harryadel
Copy link
Contributor

harryadel commented Feb 16, 2022

The dependencies in fetch package are one major version behind. Should they be updated?

  "node-fetch": "2.3.0", // latest: [3.2.0](https://github.com/node-fetch/node-fetch)
  "whatwg-fetch": "2.0.4" // latest: [3.6.2](https://github.com/github/fetch#readme)
@StorytellerCZ
Copy link
Collaborator

v3 of node-fetch is ESM package only which doesn't have such a good support in Meteor for now, so we'll have to test this thoroughly.
https://github.com/node-fetch/node-fetch/releases/tag/v3.0.0
https://github.com/node-fetch/node-fetch/blob/main/docs/v3-UPGRADE-GUIDE.md

@harryadel
Copy link
Contributor Author

I didn't know that. Thanks for pointing it out. How about whatwg-fetch? Doesn't seem like it utilizes any of ESM stuff, no?

@denihs denihs added in-discussion We are still discussing how to solve or implement it Project:Fetch Fetch package labels Feb 21, 2022
@denihs
Copy link
Contributor

denihs commented Feb 21, 2022

Hi @harryadel, are you willing to create a PR to update these packages and test everything out?

I'm not sure if passing our automated tests would be enough, right @StorytellerCZ? I think the ideal would be to have an app using the fetch package as much as possible, testing different scenarios.

@StorytellerCZ
Copy link
Collaborator

@denihs passing tests would be a good first indicator that there isn't anything major broken. Given the nature of changes testing it in simple app or with one of the oauth that use fetch would be the most efficient. If things load from the package then I think things will go smoothly as then it is just making sure that they truly loaded as we expected and comparing with changelog to see if there is something that we are specifically using that we need to take into account.

@vlasky
Copy link
Contributor

vlasky commented Jun 7, 2022

I had to stop using the Meteor fetch package and switch to using node-fetch 3.2.5 in order to be able to use the insecureHTTPParser option and fix compatibility with servers running Imperva Incapsula WAF. The requests had been failing with the HPE_INVALID_HEADER_TOKEN parse error.

Is there really any good reason to not update Meteor fetch to use the latest version of node-fetch?

@StorytellerCZ
Copy link
Collaborator

@vlasky if node-fetch worked for you in a Meteor app, then we should be able to update this then.

@StorytellerCZ StorytellerCZ self-assigned this Sep 3, 2022
@StorytellerCZ StorytellerCZ added the good first issue Good first issue or something that should is nice to do. label Oct 1, 2022
@StorytellerCZ StorytellerCZ removed their assignment Sep 28, 2023
@harryadel
Copy link
Contributor Author

I guess this is solved now thanks to @jamauro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good first issue or something that should is nice to do. in-discussion We are still discussing how to solve or implement it Project:Fetch Fetch package
Projects
None yet
Development

No branches or pull requests

4 participants