Fix issue causing WebViewExt to not be removed/cleaned up on iOS

heyman commented 3 years ago

PR Checklist

[x] There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.

[ ] All existing tests are passing.

[ ] On Android

[ ] On iOS 9

[ ] On iOS 10

[ ] On iOS 11

[ ] On iOS 12+

I was not able to run the test suite using the instructions in the README (got a whole bunch of errors).

[ ] Tests for the changes are included

What is the current behavior?

Currently, the WKWebView created by WebViewExt on iOS will not be destroyed when its Page is removed by backward navigation.

One can verify this behavior by loading an HTML file/URL that does something every X second using setInterval(). For example play a sound (or maybe just trigger an event that causes Nativescript to log something to the console). When the Page is removed the JS within the WebView keeps running.

Also, see my comment in #61.

What is the new behavior?

While I was investigating this issue I found this and this, which led me to try out the changes is this PR which turned out to fix the issue!

Please note that I made my changes directly in the webview-ext.ios.js file inside node_modules/@nota/nativescript-webview-ext, and I've now copied over the two changed lines (and changing null -> null!for TypeScript syntax) for this PR. I'm very new to Nativescript, and to save myself from figuring out exactly what tns plugin install does (I suspect mostly just npm install), I took the liberty to submit this PR without actually testing it since it's a two line fix (I did test it by hot patching the JS file in node_modules, just not through the tns plugin install flow).

Fixes #61.

heyman commented 3 years ago

I currently use the following monkey patch to fix this issue without using a forked version:

import { isIOS } from "@nativescript/core";

const WebViewExt = require("@nota/nativescript-webview-ext").WebViewExt;
export class FixedWebViewExt extends WebViewExt {
    disposeNativeView() {
        if (isIOS) {
            this.wkWebViewConfiguration.userContentController.removeScriptMessageHandlerForName("nsBridge");
            this.wkWebViewConfiguration = null;
        }
        super.disposeNativeView();
    }
}

m-abs commented 3 years ago

Thank you very much for the contribution. It has been published with v7.0.2

heyman commented 3 years ago

Great! Thank you very much for a very useful plugin :)!

Notalib / nativescript-webview-ext