capacitor icon indicating copy to clipboard operation
capacitor copied to clipboard

RFC: Built-in Http module

Open thomasvidas opened this issue 4 years ago • 32 comments

TL;DR?

We are planning on building a new Capacitor Plugin called NHttp based on the @capacitor-community/http plugin that patches fetch, XMLHttpRequest, and adds in new functions like download, 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 and window.XMLHttpRequest functions in a Capacitor application have the same limitations that they have in the browser. This includes, but is not limited to:

  • CORS restrictions
  • Unable to use certificate based authentication
  • Unable to take advantage of parallel requests on multiple threads

These issues can be bypassed by using libraries such as @capacitor-community/http and cordova-plugin-advanced-http that use the native android/iOS http libraries to do web requests. But this introduces other problems:

  • Third party libraries still rely on window.fetch and window.XMLHttpRequest so you still aren't taking advantage of the plugins
  • You are restricted to using the plugin's APIs rather than using the existing browser APIs
  • Http requests have to wait until the plugins have been loaded into the Capacitor runtime, which may lead to race conditions.

Both 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?

  1. Create a plugin that is bundled with @capacitor/core **that provides near 100% compatibility with fetch and XMLHttpRequest. This will be opt-in in Capacitor 3.x and the default (with an opt-out) in Capacitor 4.x.
  2. Add performance improvements with multi-threading on the native layer without compromising compatibility with existing browser APIs.
  3. Create a solid base for more advanced http functions that will be bundled into the core runtime for additions in Capacitor 4.x.

Proposal

Built in Native-Http plugin (NHttp)

With NHttp, we can patch window.fetch and window.XMLHttpRequest to use window.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.

Screen_Shot_2021-10-12_at_4 28 32_PM

In Capacitor 3.x, we will add a new capacitor.config option tentatively called "useNHttp" that will default to false. This value will default to true in Capacitor 4.x. The following code snippet demonstrates how the APIs will be patched.

if (capacitorConfig.useNHttp) {
  // Assign web fetch/XMLHttpRequest to another variable
  window.webFetch = window.fetch;
  window.webXMLHttpRequest = window.XMLHttpRequest;

  // Patch fetch and XMLHttpRequest
  window.fetch = window.Capacitor.Plugins.NHttp.fetch;
  window.XMLHttpRequest = window.Capacitor.Plugins.NHttp.XmlHttpRequest;
}

Deprecate @capacitor-community/http plugin

The @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.

// old
const options = {
  url: 'https://example.com/my/api',
};
const response: HttpResponse = await Http.get(options);

// new
const response: Reponse = await window.fetch('https://example.com/my/api')

For extended functions that browser APIs don't cover, but the @capacitor-community/http plugin does, the syntax may look something similar to this.

// old
const options = {
  url: 'https://example.com/download.pdf',
  filePath: 'document.pdf',
  fileDirectory: FilesystemDirectory.Downloads,
};

const response: HttpDownloadFileResult = await Http.downloadFile(options);

// new
const response: Response = await window.download('https://example.com/download.pdf', {
  filename: 'document.pdf',
  directory: window.Capacitor.Plugins.NHttp.getDefaultDownloadDirectory()
}

If the community would like to continue maintaining the existing @capacitor-community/http plugin, please post in the comments!

Undecided

  • 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?
  • Is 100% compatibility with XMLHttpRequest and fetch required? Should we aim for the same level of compatibility that https://github.com/github/fetch has?
  • 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?
  • 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.
  • 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)

thomasvidas avatar Oct 15 '21 19:10 thomasvidas

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 avatar Oct 15 '21 20:10 alexcroox

@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.

thomasvidas avatar Oct 15 '21 20:10 thomasvidas

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 avatar Oct 15 '21 20:10 gtbono

@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.

thomasvidas avatar Oct 15 '21 21:10 thomasvidas

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.

Web API Patching Caution

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. 🙂

In Conclusion

Thanks so much for listening to our feedback! Again, I think the proposal is a great idea! 😀

Take care,

Kevin

KevinKelchen avatar Oct 16 '21 03:10 KevinKelchen

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 🤔

EinfachHans avatar Oct 16 '21 19:10 EinfachHans

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 avatar Oct 16 '21 21:10 stewones

@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 😄

thomasvidas avatar Oct 16 '21 23:10 thomasvidas

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 😅

EinfachHans avatar Oct 17 '21 09:10 EinfachHans

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.

wfairclough avatar Oct 17 '21 21:10 wfairclough

@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:

  • Cookies should work out of the box. We're exploring different ways to patch 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.
  • For multi-threading APIs, we don't have anything specific chosen right now. Right now the @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!

thomasvidas avatar Oct 18 '21 17:10 thomasvidas

Some more thoughts:

  • Canceling Requests (Cordova Plugin supports that)
  • Timeout & Retrys
  • Web-Support if using Plugin directly
  • Data serializer, to be able to send json as well as formdata etc

EinfachHans avatar Oct 18 '21 20:10 EinfachHans

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?

RafaelKr avatar Nov 30 '21 17:11 RafaelKr

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.

thomasvidas avatar Dec 02 '21 16:12 thomasvidas

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.

LifeIsStrange avatar Feb 09 '22 18:02 LifeIsStrange

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"

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

RafaelKr avatar Feb 10 '22 12:02 RafaelKr

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

LifeIsStrange avatar Feb 11 '22 18:02 LifeIsStrange

So, when in v3.x can we test it?

muuvmuuv avatar Mar 29 '22 08:03 muuvmuuv

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);

muuvmuuv avatar Mar 29 '22 20:03 muuvmuuv

Any updates on the progress of this RFC? Would love to test it out :)

p-v-d-Veeken avatar May 30 '22 11:05 p-v-d-Veeken

@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

stewones avatar Jun 01 '22 16:06 stewones

@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.

thomasvidas avatar Jun 01 '22 18:06 thomasvidas

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 avatar Jun 13 '22 02:06 theswerd

@theswerd I believe they can use conditional exports for compat, cf oh-my-fetch https://github.com/ionic-team/capacitor/issues/5145#issuecomment-1036473512

LifeIsStrange avatar Jun 13 '22 10:06 LifeIsStrange

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 .

CryptoGrampy avatar Jul 01 '22 18:07 CryptoGrampy

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.

CryptoGrampy avatar Jul 01 '22 22:07 CryptoGrampy

is there any update on this and specifically the patchfetch idea?

theswerd avatar Jul 31 '22 21:07 theswerd

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 😄

thomasvidas avatar Aug 01 '22 14:08 thomasvidas

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 avatar Aug 13 '22 10:08 malte94

@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.

ptmkenny avatar Aug 13 '22 11:08 ptmkenny