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

Axios Fails on RNW (Add support for useIncrementalUpdates in Networking Module) #2460

Closed
stecrain opened this issue May 13, 2019 · 15 comments · Fixed by #10933
Closed

Axios Fails on RNW (Add support for useIncrementalUpdates in Networking Module) #2460

stecrain opened this issue May 13, 2019 · 15 comments · Fixed by #10933
Assignees
Labels
Milestone

Comments

@stecrain
Copy link
Contributor

Any http requests from a react native app which request progress updates will currently fail.

We currently use axios for http requests, and the library requests progress updates.
https://github.com/axios/axios/blob/503418718f669fcc674719fd862b355605d7b41f/lib/adapters/xhr.js

@stecrain stecrain added the vnext label May 13, 2019
@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label May 13, 2019
@kmelmon
Copy link
Contributor

kmelmon commented May 14, 2019

@stecrain Is this needed before our June 30 milestone?

@stecrain
Copy link
Contributor Author

@kmelmon No, for now we can get our network requests working by monkey patching RCTNetworking.sendRequest to always pass down false for incrementalUpdates.
https://microsoft.visualstudio.com/Xbox.Apps/_git/Xbox.Apps.Store.RNPrototype/pullrequest/3262449?_a=files&path=%2FApp.tsx

We don't have application logic that depends on this, a library that we use requests progress events so that it can fail early.

I am much more interested in issue #2466.
Without something to fix that issue we will wind up consuming a fork of react-native-windwos to make progress on our prototyping.

@harinikmsft harinikmsft removed the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label May 22, 2019
@harinikmsft harinikmsft added this to the vNext Milestone 3 milestone May 22, 2019
@namrog84
Copy link
Contributor

namrog84 commented Oct 17, 2019

@harinikmsft, Partner(Xbox) has a use-case for this. So want to +1 it for the prioritization in Milestone 4.

"useIncrementalUpdates in Networking"

edit:
We no longer have a use-case for this. We redesigned our code to not need this anymore.

@harinikmsft harinikmsft added API: Completion must-have p1 Partner: Stream (label may be applied by bot based on author) labels Oct 29, 2019
@marlenecota
Copy link
Contributor

@chrisglein
Copy link
Member

Per comment above about partner being unblocked, downgrading priority of this.

@lukago
Copy link

lukago commented Feb 9, 2020

is react-native-windows at all development ready? Axios is a standrad library that is present in most react projects and here it is not supported since almost one year.
There should be section in readme what are the current limitations of this project compared to react native for ios/android.

@kmelmon kmelmon added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Feb 10, 2020
@YuliKl YuliKl added must-have p1 and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Feb 10, 2020
@YuliKl
Copy link
Contributor

YuliKl commented Feb 10, 2020

@marlenecota looks like Axios is a fundamental module and we should fix this sooner rather than later.

@chrisglein
Copy link
Member

Still an important bug, but we're told this is no longer immediately blocking for partners. Downgrading from 'must-have' but leaving as potential for 0.62

@chrisglein chrisglein modified the milestones: 0.63, Backlog Jul 13, 2020
@ghost ghost added the Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) label Sep 17, 2020
@chrisglein chrisglein added enhancement and removed Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) labels Sep 23, 2020
@marlenecota marlenecota removed their assignment Sep 28, 2020
@NickGerleman NickGerleman self-assigned this Oct 25, 2021
@NickGerleman NickGerleman added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Oct 25, 2021
@NickGerleman
Copy link
Collaborator

Reflagging this for triage due to severity.

@NickGerleman NickGerleman changed the title Add support for useIncrementalUpdates in Networking Module Axios Fails on RNW (Add support for useIncrementalUpdates in Networking Module) Oct 26, 2021
@NickGerleman
Copy link
Collaborator

Issue affects UWP http stack. Does not affect devmain stack.

Talking to Julio, devmain stack is very different currently, and cannot be easily ported. Convergence is a potential later project. Makes sense to fix uwp implementation separately in the meantime.

@chrisglein
Copy link
Member

Reflagging this for triage due to severity.

Can you provide more detail? Last update was from over a year ago.

@chrisglein chrisglein added Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Oct 28, 2021
@NickGerleman
Copy link
Collaborator

We forgot to implement a pretty simple callback and it breaks using the most popular http library in the JS ecosystem.

@asklar asklar removed the Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) label Nov 1, 2021
@asklar
Copy link
Member

asklar commented Nov 1, 2021

Looks like there isn't new customer requests for this, is there any urgency to implement this?

@NickGerleman
Copy link
Collaborator

It's an issue that will hit many/most real projects out there. The determined folks can work around it (and several have, from the above comments), but we shouldn't be requiring folks to monkey-patch internal networking code to get correct enough behavior to use the defacto JS HTTP client.

Just from the significance, it feels like having a platform that advertises "Run your C-Sharp code here!", but you had a bug that meant Newtonsoft didn't work.

The specific issues looked like it was missing implementation of 30 LOC on other platforms, for something that people repeatedly hit.

@JunielKatarn
Copy link
Contributor

Incremental updates is not currently supported in Microsoft.ReactNative.

We will open the corresponding issue for follow up.

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

Successfully merging a pull request may close this issue.