nordnet / cordova-hot-code-push

[DEPRECATED] - This plugin provides functionality to perform automatic updates of the web based content in your application.
https://github.com/nordnet/cordova-hot-code-push/issues/371
MIT License
979 stars 467 forks source link

Does not work with cordova-plugin-wkwebview since 1.2.0 #127

Closed andreialecu closed 6 years ago

andreialecu commented 8 years ago

I haven't updated in a while, and just tried doing so and ran into a problem.

There is some incompatibility in the way https://github.com/nordnet/cordova-hot-code-push/issues/47 was implemented which makes the web server used by the wkwebview plugin not be able to serve the files from "external storage".

The initial startup goes fine, but on the second start of the app it hangs on the splash screen and nothing loads. I connected Safari to the device and can see that the request for index.html returns a 404 from wkwebview's webserver.

The difference is that pre 1.2.0 the webview was being set to an url like http://localhost:12334/Application%20Support/..../www/index.html while the new version attempts to override the wwwFolder and simply set the location to /index.html.

Would it be feasible to revert to the old behavior? Is there any benefit from the new one?

andreialecu commented 8 years ago

Also note that https://github.com/Telerik-Verified-Plugins/WKWebView/ does not support cordova-ios > 4, so I'm currently using 3.9.1.

For now, I reverted to 1.1.2 of CHCP which seems to not have this issue.

andreialecu commented 8 years ago

Actually, I just tried 1.2.0 again and it worked. I kept installing newer versions until I found the one where this was broken. 1.2.1 works, but 1.2.2 doesn't.

nikDemyankov commented 8 years ago

The difference is that pre 1.2.0 the webview was being set to an url like http://localhost:12334/Application%20Support/..../www/index.html while the new version attempts to override the wwwFolder and simply set the location to /index.html. Is there any benefit from the new one?

Yes, there are benefits from the new approach. You can look into documentation.

Actually, I just tried 1.2.0 again and it worked. I kept installing newer versions until I found the one where this was broken. 1.2.1 works, but 1.2.2 doesn't.

That's an interesting observation. Difference between these versions should not be big, just some bug fixes. Maybe I "fixed" something else as well...

andreialecu commented 8 years ago

Actually I think there may have been some caching going on as much as I tried to avoid it. 1.2.0 on the physical device didn't work, but in the simulator it did. I think it was still using the old version somehow as much as I tried to avoid that.

I reverted to 1.1.2 for now.

andreialecu commented 8 years ago

Yes, there are benefits from the new approach. You can look into documentation.

Not sure I see the part that refers to this in the documentation. The new separate folders per release feature is great, but it doesn't refer to the wwwFolder issue.

From what I can see while debugging the plugin attempts to set the wwwFolder directly to the Application Support release folder, and load /index.html from it and this is somehow incompatible with the cordova server plugin.

If it simply updated window.location to the Application Support/... folder like it did before 1.2.0 there wouldn't have been any problem.

It may be a bug in the cordova webserver plugin, I'm not denying that, but since the previous method worked, I'm just wondering why it was changed.

Edit:

But on every next launch/update - we will load an index page from the external storage.

Found just this, no explanation though as to why that is a better approach. :)

nikDemyankov commented 8 years ago

Sorry for the late reply.

Found just this, no explanation though as to why that is a better approach. :)

Yeah, but if you continue reading:

For each release plugin creates a folder with this name on the external storage, and puts in it all your web files. It is a base url for your project. This approach helps to solve several problems:

  • Files caching issue. For example, on iOS css files are cached by the UIWebView, and even if we reload the index page - new styles were not applied. You had to kill the app in the task manager to flush it, or do some hacks to change the url of the css file.
  • Less chances that update will corrupt the existing content, since we are using totally different folders for each release.
  • But if it is corrupted - we can rollback to the previous version.

So, the main idea behind "new release -> new folder" approach is that it makes plugin more stable and less chances of breaking the app, since releases are now more independent.

I think soon chcp will support cordova-plugin-wkwebview-engine plugin. Then will try to make it work with https://github.com/Telerik-Verified-Plugins/WKWebView/.

andreialecu commented 8 years ago

I think you are misunderstanding what I mean. The problem is not with the different release folders but with the way it sets the starting wwwfolder on subsequent app starts.

The older versions would redirect to a deep folder via javascript, the new versions try to set it from the backend and simply load /index.html

nikDemyankov commented 8 years ago

The older versions would redirect to a deep folder via javascript

No, it was always reloading index page from the native side. Even in older versions.

The problem is not with the different release folders but with the way it sets the starting wwwfolder on subsequent app starts.

If I remember correctly, I had to switch to setting wwwFolder instead of startPage because of the issue with having # in the index.html path - https://github.com/nordnet/cordova-hot-code-push/commit/ccdaffedbdda60df51fa25fee95e4b82fa0c3ba2.

Can double-check it later. But I suspect, that fix for telerik plugin will be similar to cordova-plugin-wkwebview-engine: set for the local server www folder as /some/path/Library/Application Support/<PACKAGE_NAME>/cordova-hot-code-push-plugin/.

andreialecu commented 8 years ago

That is it then. With startPage it used to work.

barocsi commented 8 years ago

So for Telerik WKWebview users is this a better approach to revert to 1.1.2 or 1.2.0? Is there any breaking change in later releases?

barocsi commented 8 years ago

I have reverted to 1.1.2 despite using custom load options file seems to be xcompatible with 1.3. @nikDemyankov are there any other issues I should be aware with 1.1.2 until you (if have some more energy) can bring in WKWebview Telerik version alive? Thanks

nikDemyankov commented 8 years ago

@barocsi You can look into changelog or in closed milestones to see, what has changed since v1.1.2. I can try to make a branch, that might fix this issue, but it will work only if index page doesn't contain # in url.

nikDemyankov commented 8 years ago

@andreialecu If you want - you can try out current version with telerik wkwebview. To do that - use the following command to add plugin:

cordova plugin add https://github.com/nordnet/cordova-hot-code-push.git#telerik-wkwebview-fix

It should work, if your index page does not have # in it's url.

andreialecu commented 8 years ago

The branch works. Thanks!

nikDemyankov commented 8 years ago

Glad to hear that :)

barocsi commented 8 years ago

is this merged in the new release?

nikDemyankov commented 8 years ago

@barocsi Branch telerik-wkwebview-fix is not merged into the master, since there is still a problem with #, if it is used in the index page. But I can merge master into that branch, so you could use it, if needed. But be advised, that this is a fix for telerik version of wkwebview, not the https://github.com/apache/cordova-plugin-wkwebview-engine .

nikDemyankov commented 8 years ago

Merged master into telerik-wkwebview-fix branch.

kristfal commented 8 years ago

@nikDemyankov Thanks for creating the telerik-wkwebview-fix, it works just as expected.

I'm wonder, could you merge the latest master into that branch again? In particular, getVersionInfo is missing, and it would be awesome to have that the branch without doing a personal fork of the branch.

nikDemyankov commented 8 years ago

@kristfal sure, will do :)

nikDemyankov commented 8 years ago

@kristfal merged and pushed the changes. Please, check it out. Should work, but I could have miss something...

nordnet-deprecation-bot commented 6 years ago

👋 Hi! Thank you for your interest in this repo.

đŸ˜ĸ We are not using nordnet/cordova-hot-code-push anymore, and we lack the manpower and the experience needed to maintain it. We are aware of the inconveniece that this may cause you. Feel free to use it as is, or create your own fork.

🔒 This will now be closed & locked.

ℹī¸ Please see #371 for more information.