Closed thomasvidas closed 2 years ago
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 XMLHttpRequest
so in theory you could keep using axios like normal and get the benefits of NHttp; the biggest one being lack of CORS errors and better performance.
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 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.
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!
Should we bring in external dependencies or use the built in Http libraries? Is the community ok with bringing in something like OkHttp or AlamoFire?
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. 🙂
Is 100% compatibility with XMLHttpRequest and fetch required? Should we aim for the same level of compatibility that https://github.com/github/fetch has?
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.
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.
Should we provide a harder opt-out that allows you to uninstall the NHttp module? Is having a passive Http module as a default a deal-breaker for some applications?
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!
How would NHttp work on the web if we extended fetch for users that are using Capacitor on Web as well as Android and iOS.
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?
Are there any other modules that should be a part of core? Cookies, motion, haptics, etc? (Certain modules like Geolocation, Push Notifications, etc can't be included in core due to permissions)
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.
With 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. 🙂
Thanks 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 NHttp
name. IMHO everything could still being addressed in the @capacitor-community/http
plugin which is already super awesome
@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 WebView
plugin directly with core; so adding the existing community Http plugin to core, refactoring it to cleanly patch in fetch
, and enabling it by default wouldn't be a radical new thing.
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 withCredentials: true
on an Angular HttpClient request will the cookie be sent to the server still?
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:
document.cookie
but it is a difficult problem to solve since, on iOS, the WKWebView code we use is on a separate process than the Capacitor application, which makes synchronous APIs nearly impossible. But we are aware its a fix/feature we need to add. But ultimately yes, we want it to be a drop-in replacement and developers don't even notice anything different with any http library they use.@capacitor-community/http
requests use AsyncTask
on Android and nothing besides UrlSession
on iOS. But I'd imagine we'd create a discrete DispatchQueue
or DispatchGroup
for iOS and use a similar approach on Android. We haven't really thought about allowing async/thread management from JS and that's maybe a slippery slope, but definitely something we should look into that we haven't yet!Some more thoughts:
Is 100% compatibility with XMLHttpRequest and fetch required? Should we aim for the same level of compatibility that github/fetch has?
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 fetch
or XMLHttpRequest
Especially as the capacitor documentation states as first sentence in its introduction: "Capacitor provides a consistent, web-focused set of APIs that enable an app to stay as close to web standards as possible"
If you want to provide advanced features you should provide another function (or add a function on the prototype).
Maybe another good solution would be to provide a Fetch
factory to configure a preconfigured client instance. There you can define default headers for things like authentication which then is used on every request called with this client instance. You also would then be able to have completely separated client instances, which have different default, e.g. http certificate pinning per client. You could add a CookieJar
factory to create seperate cookie instances that you can pass to a client to even have completely separated cookie sessions or pass one CookieJar
to multiple clients to have shared cookie pools (Maybe you can have a default cookie jar which is shared with document.cookie
)
Should we provide a harder opt-out that allows you to uninstall the NHttp module? Is having a passive Http module as a default a deal-breaker for some applications?
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 smiley
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:
Plugin | Description | Docs | |
---|---|---|---|
[ ] | @capacitor/native-http | Patch fetch and XMLHttpRequest for performance improvements with multi-threading on the native layer | [Link to docs] |
IMO it shouldn't be ticked by default (opt-in), but the user can quickly decide which plugins he likes to add.
@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.
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?
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 -> https://github.com/nodejs/node/commit/6ec225392675c92b102d3caad02ee3a157c9d1b7 Therefore it would make sense for capacitor to leverage it and to allocate human resources at optimizing the node implementation vs a duplicated implementation with divided human resources. As always benchmarks are welcome.
Is 100% compatibility with XMLHttpRequest and fetch required? Should we aim for the same level of compatibility that github/fetch has?
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
fetch
orXMLHttpRequest
Especially as the capacitor documentation states as first sentence in its introduction: "Capacitor provides a consistent, web-focused set of APIs that enable an app to stay as close to web standards as possible"
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 XMLHttpRequest
or fetch
breaks. Extending any of the passed object arguments to have additional functionality would be fine for me.
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 @capacitor-community/http
since it got published because we need the native performance and stuff like SSL pinning. The idea to patch fetch
/XHR
is pretty neat because in case of Angular, we can go ahead and use outstanding HTTP plugins like @ngneat/cashew
and don't need to reimplement the wheel with a lot lot lot of hacking around @capacitor-community/http
.
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 bundledWebRuntime
, so we could just say patchHttp
. Or use a function like @ionic/pwa-elements
have with defineCustomElements
, which would give it more flexibility:
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 ? ie: I need to pass a session delegate from DataDog to monitor requests sent from the URLSession instance as resources
@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 https://github.com/ionic-team/capacitor/issues/5145#issuecomment-1036473512
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). Should be already available: https://github.com/ionic-team/capacitor/releases/tag/4.3.0
Thank you! @ermannos That's great.
If you are using httpProxyMiddleware, this doesn't seem to be working currently.
const { createProxyMiddleware } = require("http-proxy-middleware");
module.exports = function (app) {
app.use(
'/api',
createProxyMiddleware({
target: 'http://localhost:3001/',
changeOrigin: true,
})
);
};
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.
if (isPlatform('ios') || isPlatform('android')) {
axios.defaults.baseURL = "https://productionserver.com/";
}
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?
document.cookie = "auth=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/;";
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 document.cookie
API and allow you full functionality that you would expect there.
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! Happy Coding!
Official announcement here: youtube.com/watch?v=NLHF3e4eMtA (30:28). Should be already available:
4.3.0
(release)
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. @capacitor-community/http had same problem i wrote an issue but it was forgotten.... I think Decoding could be handled in HttRequestHandler class readData function. For example:
static Object readData(ICapacitorHttpUrlConnection connection, ResponseType responseType) throws IOException, JSONException {
InputStream errorStream = connection.getErrorStream();
String contentType = connection.getHeaderField("Content-Type");
InputStream stream = (errorStream != null ? errorStream : connection.getInputStream());
String encoding = connection.getHeaderField("Content-encoding");
if (encoding != null && encoding.contains("gzip")) {
stream = new GZIPInputStream(stream);
}
if (errorStream != null) {
if (isOneOf(contentType, MimeType.APPLICATION_JSON, MimeType.APPLICATION_VND_API_JSON)) {
return parseJSON(readStreamAsString(errorStream));
} else {
return readStreamAsString(errorStream);
}
} else if (contentType != null && contentType.contains(MimeType.APPLICATION_JSON.getValue())) {
// backward compatibility
return parseJSON(readStreamAsString(stream));
} else {
switch (responseType) {
case ARRAY_BUFFER:
case BLOB:
return readStreamAsBase64(stream);
case JSON:
return parseJSON(readStreamAsString(stream));
case DOCUMENT:
case TEXT:
default:
return readStreamAsString(stream);
}
}
}
@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: https://github.com/capacitor-community/http/pull/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.