shripalsoni04 / nativescript-webview-interface

Plugin for bi-directional communication between webView and android/ios
MIT License
89 stars 35 forks source link

NativeScript WebView migrates to WKWebView #22

Closed surdu closed 6 years ago

surdu commented 6 years ago

Starting with tns-core-modules v3.4.0 NativeScript will migrate to WKWebView instead of UIWebView.

This change completely breaks this plugin:

JS ERROR TypeError: this.webView.ios.stringByEvaluatingJavaScriptFromString is not a function. 
(In 'this.webView.ios.stringByEvaluatingJavaScriptFromString(strJSFunction)', 'this.webView.ios.stringByEvaluatingJavaScriptFromString' is undefined)
wongwingho commented 6 years ago

An band-aid fix that seems to work, if you override _executeJS.

evaluateJavaScriptCompletionHandler seems to be an async function though, not sure how to best handle the return value for this plugin.

        if (app.ios) {
            webViewInterfaceModule.WebViewInterface.prototype._executeJS = function(strJS) {
                this.webView.ios.evaluateJavaScriptCompletionHandler(strJS, (val, err) => {console.log(val); });

                return "";
            };
        }
dhananjaykumar880 commented 6 years ago

I am facing same problem and @wongwingho, your solution is not working. can we have another workaround? I want to access height and cookie of the web view. help me

hettiger commented 6 years ago

@wongwingho Listeners still don't seem to fire correctly. We need a real fix, this makes it impossible to move forward to {NS} 3.4

wongwingho commented 6 years ago

@hettiger Upon a quick look it seems like the problem happens because of my band-aid fix of _executeJS returns an empty string, which WebViewInterface.prototype._interceptCallsFromWebview() needs a return value from the _executeJS call to handle events.

The crux of the problems stems from evaluateJavaScriptCompletionHandler needs a handler for return value as it is aysnc.

I have tested a simple event call from webview to native code with the following workaround as a proof of concept.

However this still leaves _executeJS without an return value.

nativescript-webview-interface/index.ios.js:24

    WebViewInterface.prototype._interceptCallsFromWebview = function (args) {
        var request = args.url;
        var reqMsgProtocol = 'js2ios:';
        var reqMsgStartIndex = request.indexOf(reqMsgProtocol);
        if (reqMsgStartIndex === 0) {
            var reqMsg = decodeURIComponent(request.substring(reqMsgProtocol.length, request.length));
            var oReqMsg = common.parseJSON(reqMsg);
            if(oReqMsg){ //Replaced this block
                let aysncExec = () => {
                    var eventName = oReqMsg.eventName;
                    var strJS = 'window.nsWebViewInterface._getIOSResponse('+oReqMsg.resId+')';
                    this.webView.ios.evaluateJavaScriptCompletionHandler(strJS, (val, err) => {
                        // let data = this.val;
                        this._onWebViewEvent(eventName, val);

                    });
                };
                aysncExec();
            }
        }
    };
hettiger commented 6 years ago

@wongwingho Thank you for your help. I've got a working solution now I guess:

const parseJSON = function (data) {
    let oData;
    try {
        oData = JSON.parse(data);
    } catch (e) {
        return false;
    }
    return oData;
};

(WebViewInterface.prototype as any)._executeJS = function(strJSFunction) {
    return new Promise((resolve) => {
        this.webView.ios.evaluateJavaScriptCompletionHandler(strJSFunction,
            (data) => {
                resolve(data);
            }
        );
    })
};

(WebViewInterface.prototype as any)._interceptCallsFromWebview = function (args) {
    var request = args.url;
    var reqMsgProtocol = 'js2ios:';
    var reqMsgStartIndex = request.indexOf(reqMsgProtocol);
    if (reqMsgStartIndex === 0) {
        var reqMsg = decodeURIComponent(request.substring(reqMsgProtocol.length, request.length));
        var oReqMsg = parseJSON(reqMsg);
        if(oReqMsg){
            var eventName = oReqMsg.eventName;
            this._executeJS('window.nsWebViewInterface._getIOSResponse('+oReqMsg.resId+')')
                .then(data => this._onWebViewEvent(eventName, data));
        }
    }
};
surdu commented 6 years ago

@wongwingho I didn't look too deep into what your exact problems is, but it sounds like async/await might help you out. Though, I don't know if we have access to it in NativeScript (yet) and I don't have the time to try it out.

hettiger commented 6 years ago

@surdu async / await could be cleaner but does not improve anything compared with my promise solution. Anyways if you want to try it you can do that right now I guess: https://www.nativescript.org/blog/use-async-await-with-typescript-in-nativescript-today

In case anyone needs custom local fonts too, I've got a workaround for that one as well:

const fontPath = isIOS ? 'app/fonts/' : knownFolders.documents().path + '/app/fonts/';

...

const fontFaceStyle = `
@font-face {
    font-family: Roboto-Regular;
    src: url(${fontPath + 'Roboto-Regular.ttf'});
}
`;

...

if (isIOS) {
    // This is required in order to use local fonts for example...
    const bundlePath = NSBundle.mainBundle.bundlePath;
    const bundleUrl = NSURL.fileURLWithPath(bundlePath);
    (webView.ios as WKWebView).loadHTMLStringBaseURL(html, bundleUrl);
} else {
    webView.src = html;
}

I'm using some preprocessing etc. on my html so using the constants fontPath / fontFaceStyle is no problem for me.

hettiger commented 6 years ago

Just added a PR to fix this: https://github.com/shripalsoni04/nativescript-webview-interface/pull/24

dhananjaykumar880 commented 6 years ago

@hettiger add .catch(err => {}); after .then in _interceptCallsFromWebview to solve Unhandled Promise rejection.

sometimes this error occurred

hettiger commented 6 years ago

@dhananjaykumar880 I don't think it's a good idea to just catch and leave any possible errors unhandled. If there's an error that we can properly handle please provide additional information. Don't want to be responsible for possible debugging nightmares... (I never got any errors so far...)

dhananjaykumar880 commented 6 years ago

Hi @hettiger , this error is comming

screen shot 2018-01-22 at 13 11 01

Steps to produce this error

then you will get this error and I tried but was not fixed.

I saw merged code and used but still got the same error.

and sometimes it returns this error

screen shot 2018-01-22 at 13 53 24
dhananjaykumar880 commented 6 years ago

@hettiger sometime promise resolved with null data thats why this error is comming.

I solved it by adding a condition in .then

if(data) { this._onWebViewEvent(eventName, data); }

now it is working fine. please check it on your side and if possible create new PR.

Thanks

hettiger commented 6 years ago

Thank you for the follow up @dhananjaykumar880

I will look into this when I get a chance to. It's on my todo list.
Expect feedback / another PR until next week.

hettiger commented 6 years ago

@dhananjaykumar880 I have not tried to recreate your exact environment because I would probably make something different anyways and therefore I don't think it's time well spent. I reused my own testing setup from the pull request and applied a few changes to reproduce your issue. I did fail doing so. Still no errors what so ever:

Template
<GridLayout class="page" rows="*, *">
    <WebView row="0" (loaded)="onWebViewLoaded($event)"></WebView>
    <WebView row="1" (loaded)="onWebViewLoaded($event)"></WebView>
</GridLayout>
Component
...

onWebViewLoaded(args) {
    const webView: WebView = args.object;
    const webViewInterface = new WebViewInterface(webView, '~/www/index.html');
    webViewInterface.on("nsWebViewInterfaceTest", (payload) => {
        console.log(JSON.stringify(payload));
    });
    setTimeout(() => {
        webViewInterface.emit("nsWebViewInterfaceWrite", {
            message: "Great"
        });
        webViewInterface.callJSFunction("printDate", undefined, (response) => {
            console.log(JSON.stringify(response));
        });
    }, 3000);
}

...
~/www/index.html
...

<h1 id="headline">Works!</h1>

...

<script>
    nsWebViewInterface.emit("nsWebViewInterfaceTest", { message: "works" });
    nsWebViewInterface.on("nsWebViewInterfaceWrite", function(payload) {
        document.getElementById("headline").innerText = payload.message;
    });

    function printDate() {
        var dateParagraph = document.createElement("p");
        dateParagraph.innerText = new Date().toDateString();
        document.body.appendChild(dateParagraph);
    }
</script>

...

simulator screen shot - iphone x - 2018-01-27 at 22 40 38

Console
Successfully synced application org.nativescript.test34 on device 95D46F2B-9A1E-455F-8AF9-21CDED51EDB3.
CONSOLE LOG file:///app/tns_modules/tns-core-modules/inspector_modules.js:1:82: Loading inspector modules...
CONSOLE LOG file:///app/tns_modules/tns-core-modules/inspector_modules.js:6:12: Finished loading inspector modules.
CONSOLE LOG file:///app/tns_modules/@angular/core/bundles/core.umd.js:3714:20: Angular is running in the development mode. Call enableProdMode() to enable the production mode.
CONSOLE LOG file:///app/item/items.component.js:14:24: {"message":"works"}
CONSOLE LOG file:///app/item/items.component.js:14:24: {"message":"works"}
CONSOLE LOG file:///app/tns_modules/tns-core-modules/ui/web-view/web-view.js:59:28: null
CONSOLE LOG file:///app/tns_modules/tns-core-modules/ui/web-view/web-view.js:59:28: null
CONSOLE LOG file:///app/item/items.component.js:21:28: null
CONSOLE LOG file:///app/item/items.component.js:21:28: null

As you can see in my case callJSFunction response was null as well and I'm also using multiple WebView's as you suggested. To me the plugin seems to work perfectly fine.

If you are still facing these errors, as an alternative you should be able to try {} catch (e) {} the callJSFunction call since my PR got merged.

For further investigation I would need a repository that I can just clone and run to reproduce the error.

kaurag007ph commented 6 years ago

I'm having the same issue with @dhananjaykumar880 regarding with unhandled promise rejection error after running callJSFunction. I'm using the latest 1.4.2. It happens on my real device but works with the simulator.

hettiger commented 6 years ago

@kaurag007ph For further investigation I would need a repository that I can just clone and run to reproduce the error. Can you provide such a repository?

kaurag007ph commented 6 years ago

@hettiger you can reproduce the issue using this repo https://github.com/kaurag007ph/ns-webview.git

There is no issue emitting command from webview to the app. The issue comes when you emit command or running callJSFunction from the main app to webview.

hettiger commented 6 years ago

@kaurag007ph You know that the unhandled promise rejection is actually pointing you to an error that is in your webview javascript code? (The declaration of the variable dateParagraph is missing in your code) I guess you did this intentionally to allow me reproducing these errors, right?

Here's the error I get:

CONSOLE ERROR file:///.../zone-nativescript.js:569:26: 
Unhandled Promise rejection: Error Domain=WKErrorDomain Code=4 "A JavaScript exception occurred" UserInfo={WKJavaScriptExceptionLineNumber=20, WKJavaScriptExceptionMessage=ReferenceError: 
Can't find variable: dateParagraph, WKJavaScriptExceptionColumnNumber=26, WKJavaScriptExceptionSourceURL=file:////.../app/www/index.html, NSLocalizedDescription=A JavaScript exception occurred} ; Zone: <root> ; Task: null ; Value: Error Domain=WKErrorDomain Code=4 "A JavaScript exception occurred" UserInfo={WKJavaScriptExceptionLineNumber=20, WKJavaScriptExceptionMessage=ReferenceError: Can't find variable: dateParagraph, WKJavaScriptExceptionColumnNumber=26, WKJavaScriptExceptionSourceURL=file:////Users/martin/Library/Developer/CoreSimulator/Devices/95D46

I do think it is a bad idea to silence these errors by simply adding a catch block without proper error handling to the promise. We have already discussed this here: https://github.com/shripalsoni04/nativescript-webview-interface/pull/24/files/384b0f1daab1149a8dc61e9f38142ad6ca84510f#r162748609

As you can see in this case the unhandled promise rejection error is really helpful... Silencing it would make it more difficult to spot the error.

@surdu I think the discussed solution was a bad idea. I just tested it – try catch will do nothing in this case. If I'm wrong, please show us how to. I'm basically wrapping everything inside the onWebViewLoaded method into a try catch block. Still the unhandled promise rejection error will be printed to the console. I must admit that I thought it would be possible to catch these. I guess I've learned something new today.

Here's a quick proof that try catch won't work:

try { new Promise(() => { throw new Error('exception!'); }); } catch (error) {}

Result: Uncaught (in promise) Error: exception!

try { new Promise(() => { throw new Error('exception!'); }).catch(error => { throw error; }); } catch (error) {}

Result: Uncaught (in promise) Error: exception!

try { new Promise(() => { throw new Error('exception!'); }).catch(error => {}); } catch (error) {}

Result: Silence

You can try these in the console of a modern browser...

I think a proper solution would be to return Promises on the public api methods so we could catch errors here or do something on success. Example:

webViewInterface
    .emit("nsWebViewInterfaceWrite", {
        message: "Great"
    })
    .then((data) => {
        // do something here
    })
    .catch((error) => {
        // handle error here
    });

Another option, that I personally don't like, would be to remove my catch block and stop rejecting on errors. Instead we could console log that javascript errors occurred within the webview. (I don't think that's better than the unhandled promise rejection but I guess I'm opinionated...)

Or of course how it has already been suggested: Silence the errors. But I think that is an anti pattern and I refuse to contribute such code.

@shripalsoni04 @surdu @kaurag007ph @dhananjaykumar880 What do you guys think about this? Which implementation would you prefer @shripalsoni04 – after all it's your plugin.

shripalsoni04 commented 6 years ago

Hi @hettiger , really appreciate the effort you have put in this 👍 .

I totally agree with you that we should never silence the error by writing .catch(e) {}. This will cause debugging nightmare to the users of this plugin.

I checked the code shared by @kaurag007ph and found that the error of Uncaught promise comes from _executeJS() call in emit method of index-common.js file. But as the actual error is coming from user's web-view code, I think there isn't any need to handle it inside this plugin and we can easily correct/handle web-view error as explained below:

I think there can be mainly two kind of errors that can occur in web-view code:

  1. Error due to code itself like syntax error.

    • This should be corrected while the development itself and as currently the error is being logged on console, we can easily fix it.
  2. Logic based runtime error.

    • If it is expected that some function call or events to web-view can generate error based on dynamic conditions and you need to take some actions based on that in Nativescript code, then it is better to wrap that function body/logic in try-catch block and emit the error to nativescript code in catch block as explained below:

For example in below code dateParagraph is undefined because it does not exist in HTML and so it will throw error. We can easily capture such error using try-catch block in web-view code and emit error event.

in www/index.html

nsWebViewInterface.on("nsWebViewInterfaceWrite", function (payload) {
           var dataParagraph = document.getElementById('paragraph');
            try {
                dateParagraph.innerText = new Date().toDateString();
                document.body.appendChild(dateParagraph);
            } catch(e)  {
                nsWebViewInterface.emit("webViewError", { message: e.message , stack: e.stack});
            }
 });

And then in nativescript code, you can capture this event as normal

webViewInterface.on("webViewError", (payload) => {
   console.log(JSON.stringify(payload));
});

One more thing, even if there is an unhandled promise error logged on console, the bi-directional communication will work just fine for any futher event/function calls after that error. So I think there is nothing to worry about that error getting logged on console.

@hettiger , Regarding your suggestion of returning Promise from emit method, I think if we return promise then it will create confusing API because emit method is meant to just emit an event to web-view and it is not expected to return any value. And even if we return a Promise, its success handler is of no use and it will confuse some users.

@kaurag007ph , @dhananjaykumar880 Hope this will help you in handling error in your code.

Thanks.

hettiger commented 6 years ago

@shripalsoni04 Everything you said makes perfect sense to me and I agree, that returning Promises would lead to confusion. We should leave it as is 👍

Thank you!