catalinmiron / react-native-plaid-link

React Native Plaid authenticator
124 stars 55 forks source link

feat: migrate to react-native-webview #22

Closed ossareh closed 5 years ago

ossareh commented 5 years ago

As part of the Lean Core initiative react-native are removing a range of tools that have been baked into the core until now. WebView is one of those. This change migrates to the new replacement implementation and updates the installation instructions to match.

I've tested this by publishing it to @ossareh/react-native-plaid-link v 1.3.9 on npm and tested it against my own product on android and ios.

ossareh commented 5 years ago

@catalinmiron ping (I appreciate your probably busy, but I didn't want you to have just missed this /shrug)

richeterre commented 5 years ago

Hi @ossareh, I just found this PR while trying to upgrade my home-grown Plaid webview wrapper (very similar to this project) to react-native-webview.

However, I noticed that when simply changing

import { WebView } from "react-native";

to

import { WebView } from "react-native-webview"; // version 5.8.1

my web view's onMessage handler is no longer called on success or exit. Did you experience this too?

richeterre commented 5 years ago

Okay, I found the reason why this is happening in the v5 release notes of react-native-webview:

Basically it introduces a breaking change where onMessage has to be called differently from the website. Since Plaid doesn't do this, one option is to inject some JS to restore the "old" behavior, as per the instructions in the release linked above.

ericlewis commented 5 years ago

@richeterre & @ossareh it should be fine to polyfill the behavior per the release notes.

richeterre commented 5 years ago

@ericlewis True, but surely that polyfill should happen inside react-native-plaid-link? Its consumers shouldn't need to care about which version of react-native-webview it uses under the hood…

ericlewis commented 5 years ago

Correct. This PR should do the polyfilling.

Sent via Superhuman iOS ( https://sprh.mn/?vip=ericlewis777@gmail.com )

On Thu, Jun 6 2019 at 2:58 AM, < notifications@github.com > wrote:

@ericlewis ( https://github.com/ericlewis ) True, but surely that polyfill should happen inside react-native-plaid-link ? Its consumers shouldn't need to care about which version of react-native-webview it uses under the hood…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub ( https://github.com/catalinmiron/react-native-plaid-link/pull/22?email_source=notifications&email_token=AAFEVR2GSK2EZPHGCMDCZP3PZCYP5A5CNFSM4G5Q6SIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXB5MZA#issuecomment-499373668 ) , or mute the thread ( https://github.com/notifications/unsubscribe-auth/AAFEVR5FOCLABFU4752SXPLPZCYP5ANCNFSM4G5Q6SIA ).

richeterre commented 5 years ago

Cool, sorry for the misunderstanding. So @ossareh, could you update the PR to inject this code

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

into the updated webview via the injectedJavaScript prop?

felipearosemena commented 5 years ago

Bump to this thread. I've running into some breaking issues after upgrading to the latests RN.

Will need to push this up to npm as well.

catalinmiron commented 5 years ago

react-native-plaid-link 1.4.0 published 🎉

Thanks everyone and sorry for being super late to this PR.

felipearosemena commented 5 years ago

@catalinmiron Thank you for this!

Did you see @richeterre 's comment. Should that snippet have made it into this PR?

catalinmiron commented 5 years ago

@felipearosemena thanks for notifying me about it. The issue is addressed here: #29 and published under react-native-plaid-link 1.4.2

Thanks!

felipearosemena commented 5 years ago

@catalinmiron 🎉 🙏

adrielklein commented 5 years ago

Yay thank you @catalinmiron