[feat] Add `cacheEnabled` prop #152

Salakar · 2018-11-20T11:52:06Z

This PR 'fixes' #128

cc @Titozzz @jamonholmgren as per linked issue above. Let me know your thoughts and if anything needs changing. I've enabled it by default as discussed with @Titozzz on slack.

Checklist

Android support
iOS support
Flow Types updated
Reference Documentation updated

Release Notes

[BREAKING] [FEAT] Added a new ~~enableCache~~ cacheEnabled prop to toggle Android & iOS webview caching behavior. This change makes caching enabled by default when previously there was no caching behavior which may cause unexpected behaviour changes in your existing implementations.

…th` & `setAppCacheEnabled`

Salakar · 2018-11-21T12:34:40Z

Updated - this should be good to go now, cc @Titozzz @jamonholmgren

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java

ghost · 2018-11-28T16:17:53Z

Hey there! Thanks for your work! Is there anything stopping this PR from getting merged (besides the conflicts now)?

Titozzz · 2018-11-28T16:19:13Z

Yeah, it needs testing, because when I tested it I did not feel the expected improvements

jamonholmgren · 2019-01-11T16:29:10Z

@Salakar Can you resolve the conflicts on this?

munsifhayat · 2019-01-21T07:10:55Z

Any updates on this one ?
Need to have this 'WebCache' enabled property.

Titozzz · 2019-01-21T10:14:53Z

@munsifhayat did you test this PR works for you ?

munsifhayat · 2019-01-21T10:23:48Z

@munsifhayat did you test this PR works for you ?
did't checked yet. Working on another task.

Titozzz · 2019-01-21T10:56:59Z

Keep me updated

christoph-jerolimov · 2019-01-21T14:23:56Z

For my project I can confirm that this PR improves the caching and performance. Thanks for this PR. I think this is a great and important improvement. 👍

@Salakar @munsifhayat @Titozzz But unfortunately, in the WebView.ios.js its required to push the props to the <NativeWebView... in line 249, simiar to the other ...enabled props:

https://github.com/react-native-community/react-native-webview/blob/ac59610f55fbe909b2120dfef029ac4b33811b60/js/WebView.ios.js#L248-L258

Without this _enableCache is always false in RNCWKWebView.m. I expect that its similar on Android but doesn't debug it yet.

I would also recommend to rename enableCache to cacheEnabled or cachingEnabled (or something similar), to match all the other ...enabled props

Titozzz · 2019-01-21T16:36:09Z

@jerolimov excellent catch about the props naming. We need to do that. 👍

Titozzz · 2019-01-23T10:31:47Z

@Salakar @jerolimov I updated the PR according to the reviews above :)
What do you think ?

Salakar · 2019-01-23T18:24:05Z

LGTM 👌

jamonholmgren · 2019-01-23T18:38:44Z

Let's ship it!

christoph-jerolimov · 2019-01-25T00:16:02Z

@Titozzz Sorry for the delay. Thanks for applying the feedback. Sourcecode looks fine. I tested it also in a live project on iOS and it works as expected. 👍

(I could not verify this on Android yet, because I have other troubles with my Android build atm.)

munsifhayat · 2019-01-25T09:23:26Z

@jerolimov
From my side I installed it got an RCTWebview error which I solved by dragging that library into Xcode project.

Imported the Webivew as required and than :
<WebView
ref="webview"
source={{ uri: this.state.url }}
onLoadEnd={e => this.webViewLoaded()}
domStorageEnabled = {true}
javaScriptEnabled = {true}
style={styles.container}
cacheEnabled = {true}
/>

Rest are working fine but 'cacheEnable' could be verified once I have live build Soon. Let me know if anything need to change.
I am using this for installing :

"react-native-webview": "git+https://github.com/react-native-community/react-native-webview.git"

christoph-jerolimov · 2019-01-25T10:00:05Z

@munsifhayat With your linked version, you referes to the latest react-native-webview. This version does not include the cacheEnabled prop until this PR got merged.

You need to refer the branch which is requested to merge here. You can define this in your package.json this way:

  ...
  "react-native-webview": "react-native-webview@invertase/react-native-webview#master"
  ...

I install this with yarn (npm install should work similar):

yarn add "react-native-webview@invertase/react-native-webview#master"

You find the username and branch of the PR in the top-right corner of the PR:

Hope this helps.

munsifhayat · 2019-01-29T11:32:40Z

Its working perfectly fine.

# [4.0.0](v3.2.2...v4.0.0) (2019-01-30) ### Features * **iOS/Android:** Add `cacheEnabled` prop ([#152](#152)) ([83ce79f](83ce79f)) ### BREAKING CHANGES * **iOS/Android:** This change makes caching enabled by default when previously there was no caching behavior which may cause unexpected behaviour changes in your existing implementations.

Titozzz · 2019-01-30T09:34:50Z

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

NeverFearTheasHere · 2019-02-11T20:58:16Z

I think this cacheEnabled prop is missing from the typescript prop type declarations (intypings/index.d.ts) so I've opened a PR for that: #335

Added a new cacheEnabled prop to toggle Android & iOS webview caching behavior. BREAKING CHANGE: This change makes caching enabled by default when previously there was no caching behavior which may cause unexpected behaviour changes in your existing implementations.

# [4.0.0](react-native-webview/react-native-webview@v3.2.2...v4.0.0) (2019-01-30) ### Features * **iOS/Android:** Add `cacheEnabled` prop ([react-native-webview#152](react-native-webview#152)) ([83ce79f](react-native-webview@83ce79f)) ### BREAKING CHANGES * **iOS/Android:** This change makes caching enabled by default when previously there was no caching behavior which may cause unexpected behaviour changes in your existing implementations.

# [4.0.0](react-native-webview/react-native-webview@v3.2.2...v4.0.0) (2019-01-30) ### Features * **iOS/Android:** Add `cacheEnabled` prop ([#152](react-native-webview/react-native-webview#152)) ([83ce79f](react-native-webview/react-native-webview@83ce79f)) ### BREAKING CHANGES * **iOS/Android:** This change makes caching enabled by default when previously there was no caching behavior which may cause unexpected behaviour changes in your existing implementations.

Salakar added 4 commits November 20, 2018 11:39

[js] add shared enableCache property, defaults to true

1b6afe0

[android] add setCacheEnabled to handle enableCache prop

2cd0ed1

[ios] add enableCache prop support

de1e6b6

[docs] add reference docs for new enableCache prop

c9aa593

Salakar changed the title ~~[feat] Add enableCache prop~~ [WIP] [feat] Add enableCache prop Nov 20, 2018

Salakar added 2 commits November 21, 2018 12:50

Merge branch 'master' into master

da90b72

[android] finalise enableCache android support, uses `setAppCachePa…

6d70dea

…th` & `setAppCacheEnabled`

Salakar changed the title ~~[WIP] [feat] Add enableCache prop~~ [feat] Add enableCache prop Nov 21, 2018

Titozzz previously approved these changes Nov 21, 2018

View reviewed changes

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java Show resolved Hide resolved

Titozzz added the Maintainer: need testing Code looks good, can someone help us and test this ? label Nov 21, 2018

Merge branch 'master' into master

9d2bba9

Salakar dismissed Titozzz’s stale review via 9d2bba9 November 28, 2018 16:27

Merge branch 'master' into master

665c802

Titozzz and others added 3 commits January 22, 2019 19:14

Merge branch 'master' into pr/152

8ec7c16

wip

1148766

Merge branch 'master' into master

c3b300d

Titozzz changed the title ~~[feat] Add enableCache prop~~ [feat] Add cacheEnabled prop Jan 23, 2019

jamonholmgren approved these changes Jan 23, 2019

View reviewed changes

jamonholmgren added Maintainer: ready to publish This PR has been reviewed and is ready to be published and removed Maintainer: need testing Code looks good, can someone help us and test this ? labels Jan 23, 2019

Merge branch 'master' into master

6f3384f

Titozzz merged commit 83ce79f into react-native-webview:master Jan 30, 2019

Titozzz added the released label Jan 30, 2019

NeverFearTheasHere mentioned this pull request Feb 11, 2019

Add cacheEnabled prop to typescript prop type declarations #335

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat] Add `cacheEnabled` prop #152

[feat] Add `cacheEnabled` prop #152

Salakar commented Nov 20, 2018 •

edited by jamonholmgren

Loading

Salakar commented Nov 21, 2018 •

edited

Loading

ghost commented Nov 28, 2018

Titozzz commented Nov 28, 2018

jamonholmgren commented Jan 11, 2019

munsifhayat commented Jan 21, 2019

Titozzz commented Jan 21, 2019

munsifhayat commented Jan 21, 2019

Titozzz commented Jan 21, 2019

christoph-jerolimov commented Jan 21, 2019 •

edited

Loading

Titozzz commented Jan 21, 2019

Titozzz commented Jan 23, 2019

Salakar commented Jan 23, 2019

jamonholmgren commented Jan 23, 2019

christoph-jerolimov commented Jan 25, 2019 •

edited

Loading

munsifhayat commented Jan 25, 2019 •

edited

Loading

christoph-jerolimov commented Jan 25, 2019 •

edited

Loading

munsifhayat commented Jan 29, 2019

Titozzz commented Jan 30, 2019

NeverFearTheasHere commented Feb 11, 2019

[feat] Add cacheEnabled prop #152

[feat] Add cacheEnabled prop #152

Conversation

Salakar commented Nov 20, 2018 • edited by jamonholmgren Loading

Checklist

Release Notes

Salakar commented Nov 21, 2018 • edited Loading

ghost commented Nov 28, 2018

Titozzz commented Nov 28, 2018

jamonholmgren commented Jan 11, 2019

munsifhayat commented Jan 21, 2019

Titozzz commented Jan 21, 2019

munsifhayat commented Jan 21, 2019

Titozzz commented Jan 21, 2019

christoph-jerolimov commented Jan 21, 2019 • edited Loading

Titozzz commented Jan 21, 2019

Titozzz commented Jan 23, 2019

Salakar commented Jan 23, 2019

jamonholmgren commented Jan 23, 2019

christoph-jerolimov commented Jan 25, 2019 • edited Loading

munsifhayat commented Jan 25, 2019 • edited Loading

christoph-jerolimov commented Jan 25, 2019 • edited Loading

munsifhayat commented Jan 29, 2019

Titozzz commented Jan 30, 2019

NeverFearTheasHere commented Feb 11, 2019

[feat] Add `cacheEnabled` prop #152

[feat] Add `cacheEnabled` prop #152

Salakar commented Nov 20, 2018 •

edited by jamonholmgren

Loading

Salakar commented Nov 21, 2018 •

edited

Loading

christoph-jerolimov commented Jan 21, 2019 •

edited

Loading

christoph-jerolimov commented Jan 25, 2019 •

edited

Loading

munsifhayat commented Jan 25, 2019 •

edited

Loading

christoph-jerolimov commented Jan 25, 2019 •

edited

Loading