Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add the ability to configure maximun concurrent HTTP connections #166

Closed
rjerue opened this issue Nov 2, 2019 · 8 comments
Closed

Add the ability to configure maximun concurrent HTTP connections #166

rjerue opened this issue Nov 2, 2019 · 8 comments
Labels
🗣 Discussion This label identifies an ongoing discussion on a subject

Comments

@rjerue
Copy link

rjerue commented Nov 2, 2019

Introduction

React native seems to have inherited the behavior of web browsers that they have a set amount of concurrent connections for certain platforms. As react native is built on top of native abstractions as opposed to browser ones. These limitations seem arbitrary and can be configured. This could allow for some apps to see an increase in performance.

The Core of It

React Native's primary targets are iOS and Android. Each of the implementations used for http requests on the two platforms support the ability move away from the default value. On iOS, this abstraction seems to be made in RCTHTTPRequestHandler.mm and on Android NetworkingModule.java.

iOS requires [configuration setHTTPMaximumConnectionsPerHost:8]; (8 can be any number) whereas android seems to need a "dispatcher" to be created.

Precedent for modifying the http behavior by platform exists in "ReactNetworkForceWifiOnly" in the iOS code, though I can't find anything similar for android. I'd assume it'd be possible to just add something to the app/build.gradle.

Discussion points

  • What does the implementation look like on android? There seems be a lot of escape hatches for the OKHTTPClient to be made.
  • What about platforms outside of iOS and Android.
  • How do we test this?
@hakonk
Copy link

hakonk commented Nov 3, 2019

My understanding is that it's easier to customize the behavior of the HTTP client on Android, as the OKHTTPClient can be provided. On iOS that is not the case, as the HTTP session configuration cannot be altered from outside of the React library.

I've suggested one approach for configuring the HTTP session on iOS in #165. I'm curious to hear what your thoughts are on that matter.

@rjerue
Copy link
Author

rjerue commented Nov 3, 2019

My understanding is that it's easier to customize the behavior of the HTTP client on Android, as the OKHTTPClient can be provided. On iOS that is not the case, as the HTTP session configuration cannot be altered from outside of the React library.

I've suggested one approach for configuring the HTTP session on iOS in #165. I'm curious to hear what your thoughts are on that matter.

It definitely looks like that providing a client is possible. Configuring the NSURLSession looks to be a good way to do that the iOS side of things.

It looks as if the android side even already has what I am looking for. I think a fantastic fix to this would be to add something under the network item (because this is where google takes me), or under a "Network" setting for iOS and android. Of course, this would require changes to be made allowing for the overrides to be provided for iOS.

In terms of allowing for the overrides, I am not too familiar with how iOS does things. In Android/Java you could realistically just provide an object that implements whatever interfaces or abstracts that RN requires. I am unsure of what the alternative is on the iOS/Objective-C/Swift side of it is. The Android/Java implementation also looks to put most of the work on the OKHTTPClient builder whereas the iOS/Obj-C side seems to have a little bit more going on.

I think actually overriding the network client creations on both sides is a better way to go and would be open to helping you document that side of things provided that there's an iOS implementation that gives the similar options to how android allows for one to just provide an OKHTTPClient

@hakonk
Copy link

hakonk commented Nov 4, 2019

Thanks for referencing the commit for the Android implementation! I too believe it would be beneficial with a similar mechanism for customizing the HTTP client.

@kelset kelset added the 🗣 Discussion This label identifies an ongoing discussion on a subject label Nov 4, 2019
@hakonk
Copy link

hakonk commented Jan 4, 2020

@kelset My understanding is suggestions and PRs that also relate to the React Native core (not just additional packages such as WebView) should be funneled through react-native-community/discussions-and-proposals. Is this correct?

@kelset
Copy link
Member

kelset commented Jan 6, 2020

yeah I would say so, but there is no like "hard ruling" tbh.

I think that ideally a PR to the main repo with the PoC of the implementation is always the best option.

@hakonk
Copy link

hakonk commented Jan 6, 2020

Alright, so when you say main repo, you mean I might as well just make a PR with the PoC to the react native repo with #165?

@kelset
Copy link
Member

kelset commented Jan 6, 2020

Yes :)

Since it's a "code PR" it's going to be easier for the maintainers and FB developers to check it more closely.

OFC I can't promise that it will be merged :) but surely would make the whole process leaner.

@hakonk
Copy link

hakonk commented Jan 6, 2020

Cool, I too believe that's a leaner approach :)

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Jun 3, 2021
Summary:
While it is possible in the React Native implementation for Android to provide a custom configuration for HTTP requests, the iOS implementation does not allow for the same customization. As the NSURLSession used for HTTP requests on iOS is configured internally, one may for instance not supply an ephemeral configuration for HTTP requests. Other concerns related to the given problem have been addressed in the community: react-native-community/discussions-and-proposals#166. I did make a PR with an RFC in the community repo, but after some discussion in the said repo, I figured I might as well make a PR with a suggestion :)

## Changelog

[iOS] [Added] - Allow for configuring the NSURLSessionConfiguration

Implement a C function `RCTSetCustomNSURLSessionConfigurationProvider` which gives the app programmer the ability to provide a block which provides an NSURLSessionConfiguration that will be used for all HTTP requests instead of the default configuration. The provided block will be called when the session configuration is needed.

Pull Request resolved: #27701

Test Plan: Unsure if this can be tested in any other way than uncommenting the example code in `RNTester/RNTester/AppDelegate.mm`.

Reviewed By: yungsters

Differential Revision: D28680384

Pulled By: JoshuaGross

fbshipit-source-id: ae24399955581a1cc9f4202f0f6f497bfe067a5c
Titozzz pushed a commit to Titozzz/react-native that referenced this issue Jun 7, 2021
Summary:
While it is possible in the React Native implementation for Android to provide a custom configuration for HTTP requests, the iOS implementation does not allow for the same customization. As the NSURLSession used for HTTP requests on iOS is configured internally, one may for instance not supply an ephemeral configuration for HTTP requests. Other concerns related to the given problem have been addressed in the community: react-native-community/discussions-and-proposals#166. I did make a PR with an RFC in the community repo, but after some discussion in the said repo, I figured I might as well make a PR with a suggestion :)

## Changelog

[iOS] [Added] - Allow for configuring the NSURLSessionConfiguration

Implement a C function `RCTSetCustomNSURLSessionConfigurationProvider` which gives the app programmer the ability to provide a block which provides an NSURLSessionConfiguration that will be used for all HTTP requests instead of the default configuration. The provided block will be called when the session configuration is needed.

Pull Request resolved: facebook#27701

Test Plan: Unsure if this can be tested in any other way than uncommenting the example code in `RNTester/RNTester/AppDelegate.mm`.

Reviewed By: yungsters

Differential Revision: D28680384

Pulled By: JoshuaGross

fbshipit-source-id: ae24399955581a1cc9f4202f0f6f497bfe067a5c
tido64 pushed a commit to facebook/react-native that referenced this issue Jun 8, 2021
Summary:
While it is possible in the React Native implementation for Android to provide a custom configuration for HTTP requests, the iOS implementation does not allow for the same customization. As the NSURLSession used for HTTP requests on iOS is configured internally, one may for instance not supply an ephemeral configuration for HTTP requests. Other concerns related to the given problem have been addressed in the community: react-native-community/discussions-and-proposals#166. I did make a PR with an RFC in the community repo, but after some discussion in the said repo, I figured I might as well make a PR with a suggestion :)

## Changelog

[iOS] [Added] - Allow for configuring the NSURLSessionConfiguration

Implement a C function `RCTSetCustomNSURLSessionConfigurationProvider` which gives the app programmer the ability to provide a block which provides an NSURLSessionConfiguration that will be used for all HTTP requests instead of the default configuration. The provided block will be called when the session configuration is needed.

Pull Request resolved: #27701

Test Plan: Unsure if this can be tested in any other way than uncommenting the example code in `RNTester/RNTester/AppDelegate.mm`.

Reviewed By: yungsters

Differential Revision: D28680384

Pulled By: JoshuaGross

fbshipit-source-id: ae24399955581a1cc9f4202f0f6f497bfe067a5c
@kelset kelset closed this as completed Jun 10, 2021
@react-native-community react-native-community locked and limited conversation to collaborators Jun 10, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
🗣 Discussion This label identifies an ongoing discussion on a subject
Projects
None yet
Development

No branches or pull requests

3 participants