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

Upgrade react-native-webview to 3.x, 4.x, 5.x #3550

Closed
gnprice opened this issue Jul 9, 2019 · 2 comments · Fixed by #3564
Closed

Upgrade react-native-webview to 3.x, 4.x, 5.x #3550

gnprice opened this issue Jul 9, 2019 · 2 comments · Fixed by #3564
Assignees
Labels

Comments

@gnprice
Copy link
Member

gnprice commented Jul 9, 2019

We're using react-native-webview version 2.0.0. In the months since that release, they've made several more with major changes.

In particular @jainkuniya found with git bisect that #3518 was introduced by switching from the RN-included WebView to react-native-webview. And he found that upgrading all the way to 5.9.1 fixes #3518. (Described on #3536). AFAIK he didn't test versions in between, so it's possible a much earlier version already fixes that issue. Anyway, we should get to the latest.

The latest version of react-native-webview, from last week, comes as a pair (see NPM): 6.3.1 with AndroidX, and 5.12.1 without. Until #3548 , we'll want the non-AndroidX version, so that means the latest 5.x version.

@jainkuniya also described on #3536 finding that upgrading to 5.9.1 wasn't smooth. So let's do this in steps:

  • First, to the latest 3.x.
  • Then the latest 4.x. -> looking at the version history, 5.0.1 came less than 3 days after 4.0.0, so perhaps skip this.
  • Then the latest 5.x.

For each version:

  • Read the release notes. Figure out what breaking changes the maintainers say they made.
  • Try the upgrade.
  • Fix any issues.
  • In the commit message, include:
    • a link to the release notes
    • a mention of any breaking changes that we think might matter
    • a description of what you did about them.

A couple of things we know about already:

@gnprice
Copy link
Member Author

gnprice commented Jul 23, 2019

This appears to be a requirement for the RN v0.59 upgrade, #3399 -- versions of react-native-webview prior to 5.x say they're only compatible with RN v0.57. (A bit more discussion in #3561 .) Bumping the priority accordingly.

@gnprice
Copy link
Member Author

gnprice commented Jul 23, 2019

A tactical note: the 5.0.0 release notes say:

"""
Side note: if you wish to keep compatibility with the old version when you upgrade, you can use the injectedJavascript prop to do that:

const injectedJavascript = `(function() {
  window.postMessage = function(data) {
    window.ReactNativeWebView.postMessage(data);
  };
})()`;

"""

So that sounds like the right solution to pursue for now, to unblock the upgrade. Then we can switch to the new, said-to-be-better API at our leisure.

jainkuniya added a commit to jainkuniya/zulip-mobile that referenced this issue Jul 23, 2019
There is a complete rewrite of communication between webview and rn
component starting from 5.0.0

So use following as stated in the release notes
```
window.ReactNativeWebView.postMessage(data)
```
https://github.com/react-native-community/react-native-webview/releases/tag/v5.0.0

Also due to following change in react-native-webview/react-native-webview@f3bdab5
```
-    stringWithFormat:@"document.dispatchEvent(new MessageEvent('message', %@));",
+    stringWithFormat:@"window.dispatchEvent(new MessageEvent('message', %@));",
```

on iOS `document.addEventListener` stopped working. So change it to `window.addEventListener`.

This also fixes zulip#3550, zulip#3518.
jainkuniya added a commit to jainkuniya/zulip-mobile that referenced this issue Jul 23, 2019
There is a complete rewrite of communication between webview and rn
component starting from 5.0.0

So use following as stated in the release notes
```
window.ReactNativeWebView.postMessage(data)
```
https://github.com/react-native-community/react-native-webview/releases/tag/v5.0.0

Also due to following change in react-native-webview/react-native-webview@f3bdab5
```
-    stringWithFormat:@"document.dispatchEvent(new MessageEvent('message', %@));",
+    stringWithFormat:@"window.dispatchEvent(new MessageEvent('message', %@));",
```

on iOS `document.addEventListener` stopped working. So change it to `window.addEventListener`.

This also fixes zulip#3550, zulip#3518.
jainkuniya added a commit to jainkuniya/zulip-mobile that referenced this issue Jul 23, 2019
There is a complete rewrite of communication between webview and rn
component starting from 5.0.0

So use following as stated in the release notes
```
window.ReactNativeWebView.postMessage(data)
```
https://github.com/react-native-community/react-native-webview/releases/tag/v5.0.0

Also due to following change in react-native-webview/react-native-webview@f3bdab5
```
-    stringWithFormat:@"document.dispatchEvent(new MessageEvent('message', %@));",
+    stringWithFormat:@"window.dispatchEvent(new MessageEvent('message', %@));",
```

on iOS `document.addEventListener` stopped working. So change it to `window.addEventListener`.

This also fixes zulip#3550, zulip#3518.
gnprice pushed a commit to jainkuniya/zulip-mobile that referenced this issue Jul 24, 2019
Fixes zulip#3550.  This is also expected to fix zulip#3518.

Release notes:
  https://github.com/react-native-community/react-native-webview/releases/tag/v3.0.0
  https://github.com/react-native-community/react-native-webview/releases/tag/v4.0.0
  https://github.com/react-native-community/react-native-webview/releases/tag/v5.0.0

In v5 in particular, there were breaking changes to communication from
inside the WebView to outside and vice versa.  In one direction, we can
follow the release notes: switch to `window.ReactNativeWebView.postMessage`.

In the other direction, upstream changed the API (seemingly by accident?)
so that, on iOS only, inbound events are dispatched at `window` rather
than `document`:
```
-   stringWithFormat:@"document.dispatchEvent(new MessageEvent('message', %@));",
+    stringWithFormat:@"window.dispatchEvent(new MessageEvent('message', %@));",
```
(from react-native-webview/react-native-webview@f3bdab5a2 .)

So, add a hack to detect that and adjust accordingly.

[greg: expanded commit message; silenced lint error]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants