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

Should useWebKit be default? #69

Open
scarlac opened this issue Aug 11, 2020 · 5 comments
Open

Should useWebKit be default? #69

scarlac opened this issue Aug 11, 2020 · 5 comments
Labels
documentation Improvements or additions to documentation iOS

Comments

@scarlac
Copy link
Contributor

scarlac commented Aug 11, 2020

Question

Should we default useWebKit to true as the default for all static methods? (e.g. CookieManager.clearAll(true)) Given that react-native-webview from the community has now removed UIWebView entirely (link), it seems it would make sense to default to using WKWebView (aka useWebKit).

This would be a breaking chance, so despite 4.0 just being release it would probably to wait until 5.0 to be released.

Thoughts?

@scarlac scarlac changed the title Should useWebKit be default? Should useWebKit be default? Aug 11, 2020
@davidsharp
Copy link

I think it's the sensible default going forward, also worth noting is the caveat that setFromResponse and getFromResponse are missing this functionality altogether (#72)

@davidgovea
Copy link

I had the similar idea, but after looking into it, the "default" mode (NSHTTPCookieStorage) is used for react-native's network stack (i.e. fetch(), etc)

That would make changing the default quite a large change for anyone relying on that behavior (as opposed to just webview cookie management). Not sure if that is a common use-case, but I suppose the change would be OK if it is rare.

I actually need to manage the cookies on "both sides" in iOS, so I've been using hacks like:

for (const useWebkit of [false, true]) {
  // Call both non-webkit and webkit variants on iOS, but only once on Android
  if (!useWebkit || Platform.OS === 'ios') {
    CookieManager.set(url, cookie, useWebkit);
  }
}

To me, it would be nice to auto-sync the cookies between the two stores on iOS -- but again, not sure if my usecase is common.

@safaiyeh
Copy link
Member

Yeah the reason behind not defaulting to WebKit is that fetch is using NSHTTPCookieStorage.

There might be a cleaner design for this. Maybe even moving WebView related cookies to the WebView module #5

Looking to hear from everyone!

@scarlac
Copy link
Contributor Author

scarlac commented Aug 14, 2020

Ah that makes sense now. I think just documenting it clearly would suffice. It's not 100% clear how they are shared so i had to look into Apple docs and read source to confirm. Not the fault of this lib but I can see others being confused by it.

@safaiyeh
Copy link
Member

Sweet ill take this up as a documentation task

@safaiyeh safaiyeh added documentation Improvements or additions to documentation iOS labels Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation iOS
Projects
None yet
Development

No branches or pull requests

4 participants