-
Notifications
You must be signed in to change notification settings - Fork 1k
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
RFC: Built-in Http module #5145
Comments
The biggest thing keeping me from switching from axios to fetch still in 2021 is the lack of upload progress which is key to providing any decent UX in a modern app with file upload UI. Do you intend to support upload/download progress updates? |
@alexcroox the best part is you continue to use axios if you'd like! Under the hood, axios just uses |
Nice!! this is a game-changer feature for sure and every Capacitor developer needs this! As it is requesting for comments, I will give my two cents here but overall I think this is a very positive and needed change! :) About the OkHttp lib (Speaking as an Android developer, never done iOS) What is the main concern about bringing those types of libraries? app size? if it is I wouldn't care, IIRC react-native uses OkHttp and some iOS lib and no one complains that much, if it helps to do less buggy code, go for it!! Now my fears... one thing I fear when seeing this change is when bugs arrive, or when the NHttp fetch implementation becomes different than web's fetch. If the same code runs on web fetch and not on NHttp, it should be considered a bug in my opinion. I don't have any ideia on how you guys are thinking of implementing this, but if replicating fetch's exact behaviour in Android OkHttp and in iOS's networking lib is going to be a monumental amount of work, I'm going to try to approach the problem in another way. Is there any way we could have a synchronous bridge (similar to react native JSI and Wendux Android/iOS)? this way I believe we can still use the webview's fetch, but augment it so we can always include the certificates in the native filesystem for SSL pinning and polyfill document.cookie in iOS to use this bridge? |
@gtbono ideally we'd like to keep dependencies to a minimum. OkHttp and AlamoFire are used in thousands of apps but if there was a big pushback to not include them, then we'd find a way around it. We're aiming for near 100% compatibility with fetch and XMLHttpRequest. We want it to be a drop in replacement. So I'd agree that it would be a bug if the web fetch and the nhttp fetch were different. As for a synchronous bridge, we've talked internally about it but we don't have any plans right to support that use case; but we're aware it's a feature people would like and we might support it in the future. |
This sounds amazing, @thomasvidas! 😃💯 When I heard about the idea I wasn’t expecting the proposal to patch XHR, which would really open it up for third-party JavaScript libraries! That is a big downside of the existing plugins; a deal-breaker for solving our use case. In our case we could potentially get rid of an entire server-side proxy for a single JavaScript library that exists to work around CORS issues!
I don’t know much of anything about those libraries so don’t have much to say. As a user of Capacitor, I’m not concerned about a transitive dependency on them from, say, a native app size standpoint (unless it was really significant). If they changed Capacitor’s software licensing from MIT to something else then my ears may perk up. 😅 I would say choose the approach that makes the solution the most maintainable, less buggy, robust, etc. I’ll defer to you and the Capacitor maintainers on that. 🙂
That’s awesome! 😀 I think that will be important to make it work for third-party code! It there was a certain case where it behaved differently in third-party code that caused a bug, it’d be a shame for a user of Capacitor to have to entirely drop the plugin’s patching of the fetch and XHR APIs.
As long as it could be completely disabled if needed, I wouldn’t care if it was still bundled. I don’t think using the plugin by default is a deal-breaker. Not being able to opt-out would probably be a deal-breaker for some apps, but the proposal supports being able to opt-out so no concerns there!
Maybe there could be a runtime check to only patch fetch and XHR if it’s not Web; only do so for Capacitor platforms that have a native layer?
I think our app might have Motion and Haptics included because it was recommended to have enabled for Ionic Framework users in the Capacitor 3 upgrade doc. I don’t think I’d include them by default because non-Ionic Framework users don’t necessarily need them. I think there’s merit in keeping Capacitor Core lean. For the HTTP plugin, one could argue that making network requests is so fundamental and universal that it makes sense to include in Core. Still though, many may not need the plugin at all. What I think is a stronger argument for including it in Core is how “this allows it to be initialized at the exact time that the Capacitor bridge is initialized, allowing for all requests to use NHttp.” Given that, I think it might be a good fit for Capacitor Core. If it can remain a separate plugin and still maintain those benefits, that’s potentially another option. Web API Patching CautionWith Capacitor currently there's a zone.js and cordova.js load order issue that causes Angular change detection to not run properly. I’ve worked around it for now by using patch-package for applying the fix to @capacitor/core. Please see this issue for details. The issue involves how Web APIs are patched. We should be cautious not to introduce further problems for third-party JavaScript libraries that already patch fetch and XHR. Besides zone.js, there are JavaScript libraries we use like Sentry (error monitoring) that patch APIs like XHR. I think we can come up with something that will work, but I just wanted to draw attention to the consideration. 🙂 In ConclusionThanks so much for listening to our feedback! Again, I think the proposal is a great idea! 😀 Take care, Kevin |
That sounds amazing! I'm currently using Angular's HttpClient or (in apps where i need ssl pinning) the Cordova Plugin u mentioned. I don't know if i understand it correctly and if Angular uses the fetch stuff under the hood but doesn't matter i normaly would rewrite my code to use plugin directly 🤔 Thinks this gives a better overview for other developers about what's going on. I would like to have SSL pinning supported, that would be great. Also personally i wouldn't add this to Capacitor Core. Of course it wouldn't be a deal breaker but also it's not a deal breaker for users to install the plugin theirself. I like the fact that from V3 only the plugin you really install are within you app 🤔 |
The biggest goal of cap v3 was decouple plugins from the core, and now you want to bring plugins back to the core 😅 honestly I don’t like this idea, neither the |
@stewones, for Capacitor 3 the reason we removed the plugins because it required end users to declare permissions for Camera, Filesystem, etc even if they weren't using it. But with Http requests, the only permission required is the Internet permission, which is a silent permission. We are already bundling the That said, I'd love to hear more in depth why you (and @EinfachHans as well!) think this isn't a good idea. That's the point of the RFC 😄 |
Personally i like to have only that code in my app that is necessary, thats why i really liked when in V3 also Plugins like Haptic etc gets removed from core. I guess there are people that think the same like me and then including it directly but having an option to uninstall it sound wrong for me 😃 Or is there an advantage that i don't see from directly bundling into core? Guess for users that wants to use it a single install command would not be a deal breaker 😅 |
The idea sounds great to me and if the implementation/testing goes smoothly I feel like it would be a great addition to the Capacitor Core. I personally don't mind if OkHttp and AlamoFire are included in my apps since I usually included them anyways. Will Cookies work out of the box? When NHttp is enabled and we have What are you thinking for the multi-threading APIs? Will we have the ability to choose which thread requests run on from the JS side? I am not sure about the name NHttp, I would prefer NativeHttp or PlatformHttp to be explicit. NHttp makes me think "NoHttp" or "NotHttp" when I first read it 🤷. Anyways, I am very excited to try this out when it is released. |
@EinfachHans by bundling with core, we avoid race conditions with other libraries that patch in XHR. That is the biggest benefit with bundling directly rather than as a plugin since all of the plugins are lazily loaded; which could result in patching xhr after angular or sentry or something has already patched it. I wonder how many Capacitor developers don't use Http requests in their app 🤔, but we can always research other ways to avoid race conditions when loading scripts! @wfairclough the name isn't final, just easier to type than "Native Http" 😂 To address your comments:
|
Some more thoughts:
|
Definitely yes! When you override existing functionality it should 100% comply with it. This is required to guarantee functionality for third-party plugins that use If you want to provide advanced features you should provide another function (or add a function on the prototype).
I also like the approach of keeping the app as small as possible. So building this as opt-in plugin would be great! Maybe the capacitor install process could implement a new step where it recommends plugins like this one. It could be in a table format:
IMO it shouldn't be ticked by default (opt-in), but the user can quickly decide which plugins he likes to add.
Would it be possible to implement a functionality into capacitor that makes it possible to mark a capacitor plugin as "bundled" so that it's not loaded lazily? |
@RafaelKr at the moment, no. We are bundling a single plugin with core right now; the "WebView" plugin; which sets the base path in which web assets are served in Capacitor (pretty important stuff!). None of the other core plugins are as essential as the WebView plugin so they are loaded in lazily. This allows the app to have a faster startup time. There are other reasons we don't bundle plugins with core unless they are essential to the functionality of every Capacitor application, but that's one of the biggest ones. |
NodeJs now supports the Fetch API -> |
I think I should clarify that with "100% comply" I mean, that no existing functionality should be modified. So that no app that currently uses There's also an interesting discussion about the new NodeJS fetch api here: https://news.ycombinator.com/item?id=30161626 |
See also ohmyfetch, used by Nuxt, it leverage the library Node fetch and offer unified support for browser, Node and web workers. This is made possible by using conditional exports! https://github.com/unjs/ohmyfetch |
So, when in v3.x can we test it? |
We are using The thing about splitting and now bundling that "plugin" what @EinfachHans mentioned is a good point. I really like the way of having functionality split, but having that in core mean better compatibility for other cap plugins. I think it got mentioned already in one issue. The NHttp naming is weird, we have options like import { patchHttp } from '@capacitor/http;
patchHttp(window); |
Any updates on the progress of this RFC? Would love to test it out :) |
@thomasvidas with the new plugin bundled in the core, would we be able to pass a custom delegate to the URLSession ? |
@p-v-d-Veeken nothing to show off yet! We plan to release this as a 4.x feature (coming summer 2022!). Not 100% sure if this will be a 4.0 feature but it is a top priority! @stewones that's not on the list of features we're planning for the first release but a good idea for future iterations. |
just wanted to voice, 100% compatibility with fetch, without the limits on cross site cookies and cors would be extremely helpful for my project. |
@theswerd I believe they can use conditional exports for compat, cf oh-my-fetch |
Not sure if this is at all possible, but it would be really great if this library supported proxying requests through Tor/Orbot using something like Axios's httpAgent/httpsAgent (http.agent(?)) functionality and allowing users to pass in agents such as: https://www.npmjs.com/package/socks-proxy-agent . |
Just to add to my above comment on Tor proxying, it looks like @ProofOfKeags and Start9 labs rolled their own version of the community-http Capacitor library to add socks proxy functionality in: https://github.com/Start9Labs/capacitor-http . Again, would be great to see this added as part of the official Http module; there's a huge need for data privacy and easy ways for devs to connect apps to Tor. |
is there any update on this and specifically the patchfetch idea? |
This feature should be coming in a later 4.x version. I'm no longer part of the team, but the original plan was 4.1, so it should be coming within a few releases 😄 |
I have a theoretical question. Many libraries or external services of course do not use some Http Plugin from Capacitor or the Cookies plugin to store cookies. Nevertheless, there are many of those that also drop necessary cookies. Now when 4.1 is released: Will this all work "out of the box" or only when I use the plugins provided by Capacitor, so implement stuff myself? Thank you! :) |
@malte94 Yes, using this http plugin should effectively "transform your app into a web browser" for the purpose of handling cookies, so as long as you have the plugin enabled, you should be able to get cookies from websites in the same manner as a web browser can. Note that the release has not yet been announced, so it may not be in 4.1. |
Any news on this? Will it be ever introduced? |
Some opt-in Beta would be nice in the upcoming release to test. I have a stable Angular HTTP client with a lot of interceptors that I would love to try with it! |
Official announcement here: https://www.youtube.com/watch?v=NLHF3e4eMtA (30:28). |
Thank you! @ermannos That's great. If you are using httpProxyMiddleware, this doesn't seem to be working currently.
However, if you use axios, you can set the base.url to your server. I did this right in the index.js of my React project.
Of course axios is now pointing always to the production server. Furthermore, enable CORS within Express. Everything works fine then :) |
One more thing: It would be nice, for the application as well as for third-party-plugins, to have the ability to delete cookies. Will this be possible?
|
It seems to work quite well, e.g. using native angular http client. BUT, using a library like signalr that relies on cookie authentication when performing websocket calls does not seem to workon devices (only in web browser), since I get authentication errors. The signalr library supports passing in custom headers etc, but I don't see a way to get the cookie from capacitor. Any ideas for how to make it possible for signalr to reuse the same cookie retrieved during login with angular http client? |
Hello everyone! Thank you for taking time to share your feedback on the new Native HTTP and Cookies support. As mentioned above, this has been released with Capacitor 4.3 and is available for everyone to opt-in: // capacitor.config.ts
{
plugins: {
CapacitorCookies: {
enabled: true,
},
CapacitorHttp: {
enabled: true
}
},
} As mentioned in the docs as well as in our announcement, enabling both of these should patch the document level functions, allowing you to take full benefit of these improvements without making any changes in your code. This applies to Cookies as well, although not called out in the video, it should patch the If anyone comes across any issues with these new features, please open a new issue here on the repository and we'll be happy to take a look! As always, thanks for your continued support! |
I think this issue can now be closed and locked. Any further issues should be a new issue with proper context. Edit: xD that was fast haha |
Will the ionic team provide any official guide for SSL pinning? Will it be even supported? we have lots of our ionic apps, for which we do penetration testing, this is one of the major issue for us now. |
how to use custom interceptors like axios? |
@reslear You can use axios with the HTTP plugin, so if you want the axios custom interceptors, you can use them directly. |
Hi, Please consider adding InputStream decompression at least for GZIP encoding.
|
@d4mn Good idea! Can you make a new Issue as a feature request? This one' for the RFC. |
@d4mn we had a similar issue with the community http plugin and opened a pull request related to that, you should find all the code that is needed in the changes, we would also need this fature, since we're currently living on our own fork here: capacitor-community/http#222 |
Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out. |
TL;DR?
We are planning on building a new Capacitor Plugin called
NHttp
based on the@capacitor-community/http
plugin that patchesfetch
,XMLHttpRequest
, and adds in new functions likedownload
,upload
, and other functions. Will be opt-in in Capacitor 3 and the default in Capacitor 4.0.But please go and read! We'd love to hear the community's thoughts!
Challenge
Currently the
window.fetch
andwindow.XMLHttpRequest
functions in a Capacitor application have the same limitations that they have in the browser. This includes, but is not limited to:These issues can be bypassed by using libraries such as
@capacitor-community/http
andcordova-plugin-advanced-http
that use the native android/iOS http libraries to do web requests. But this introduces other problems:window.fetch
andwindow.XMLHttpRequest
so you still aren't taking advantage of the pluginsBoth of these approaches have drawbacks. And auth providers such as Okta require workarounds that are not intuitive. Hence this proposal!
Goal
What are we trying to achieve?
@capacitor/core
**that provides near 100% compatibility withfetch
andXMLHttpRequest
. This will be opt-in in Capacitor 3.x and the default (with an opt-out) in Capacitor 4.x.Proposal
Built in Native-Http plugin (NHttp)
With NHttp, we can patch
window.fetch
andwindow.XMLHttpRequest
to usewindow.Capacitor.Plugins.NHttp
functions in order to get the fix the challenges listed above with using the existing browser functions. This allows third party libraries to use NHttp without developers needing to rewrite their code. It also allows Capacitor developers to use more web APIs while getting the benefits of native application code/performance.NHttp will be bundled with
@capacitor/core
in a similar way that the WebView plugin is bundled/initialized with the core runtime. This allows it to be initialized at the exact time that the Capacitor bridge is initialized, allowing for all requests to use NHttp.In Capacitor 3.x, we will add a new
capacitor.config
option tentatively called "useNHttp" that will default tofalse
. This value will default totrue
in Capacitor 4.x. The following code snippet demonstrates how the APIs will be patched.Deprecate
@capacitor-community/http
pluginThe
@capacitor-community/http
plugin, which is maintained by Ionic, will be updated to support Capacitor 4.0 and then deprecated. NHttp will have feature parity with@capacitor-community/http
with the release of Capacitor 4.0. In Capacitor 5.0, the@capacitor-community/http
plugin will not be updated and may not work with future releases of Capacitor.Ionic will create a migration document for migrating code to support the
NHttp
plugin by providing code samples. An example of the migration guide would be something similar to this.For extended functions that browser APIs don't cover, but the
@capacitor-community/http
plugin does, the syntax may look something similar to this.If the community would like to continue maintaining the existing
@capacitor-community/http
plugin, please post in the comments!Undecided
OkHttp
orAlamoFire
?XMLHttpRequest
andfetch
required? Should we aim for the same level of compatibility that https://github.com/github/fetch has?NHttp
module? Is having a passive Http module as a default a deal-breaker for some applications?NHttp
work on the web if we extendedfetch
for users that are using Capacitor on Web as well as Android and iOS.The text was updated successfully, but these errors were encountered: