Prevent crash when using a NativeScript app (for alternate icon name)

[EddyVerbruggen]

After showing a Toast my app would crash when using this method.

I have very little experience in Swift so I don't know why the rootViewController getter needs to be hijacked by this pod, but to make it work for my case I'd really love to have this code merged.

[devxoul]

Can you share the way to reproduce the crash?

[EddyVerbruggen]

Good question. I just tried reproducing it with your demo and sadly for me I can't.

If I add a breakpoint to the rootViewController getter I notice that your demo will hit that method when a Toast is shown, but not when it's used inside a NativeScript app (that's my usecase).

Funny thing is that when the method to change the icon runs, both a native Swift app and a NativeScript app hit the aforementioned getter. But in the NativeScript case we need the fallback this PR was created for.

I hope that fallback doesn't bother you as it's quite similar to what you have until recently. If you're not going to merge it I'm afraid I have to use my own fork.

Btw, the screenshot below is proof that your code is compatible with the icon change feature of iOS 10.3 (unless it runs in a NativeScript app (and perhaps Cordova and React Native as well)):

Thanks for considering anyway and keep up the great work!

[devxoul]

@EddyVerbruggen I would definitely love to patch a bug if it exists. But changing rootViewController can be a critical change (because Toaster has no test 😂) so I wanted to know the exact situation. Can you reproduce the bug with a new project? (not a demo)

[EddyVerbruggen]

@devxoul I can reproduce it with a NativeScript app, not with a Swift app. Would that be of help?

[devxoul]

Hmm. Does NativeScript app have different window hierarchy from native Swift app?

[Coeur]

@EddyVerbruggen you seem to work at NativeScript.
I'll try to reproduce your issue in order to review the pull request.

I've done those install steps:

brew install node@10
echo 'export PATH="/usr/local/opt/node@10/bin:$PATH"' >> ~/.bash_profile
source ~/.bash_profile
pip install six
npm install -g nativescript

I've done those cocoapods setup steps:

tns create MYCocoaPodsApp
cd MYCocoaPodsApp
tns platform add ios
cd ..
mkdir my-plugin
cd my-plugin
touch package.json
echo '{"name":"my-plugin","version":"0.0.1","nativescript":{"platforms":{"ios":"1.3.0"}}}' > package.json
mkdir platforms
mkdir platforms/ios
touch platforms/ios/Podfile
echo "pod 'Toaster'" > platforms/ios/Podfile
cd ../MYCocoaPodsApp/
tns plugin add ../my-plugin
tns build ios
open platforms/ios/MYCocoaPodsApp.xcworkspace

Now I need to figure out how to call Toaster from javascript. I'll be back once I figure it out.

[EddyVerbruggen]

@Coeur Much appreciated! For your last comment, perhaps this can be of help: https://github.com/EddyVerbruggen/nativescript-toast/blob/master/toast.ios.js

[Coeur]

[edit] OK, next:
We add (new Toast("Hello Toaster", 0, 2.0)).show(); in main-view-model.js, and for every change in a pod we must run:

tns plugin add ../my-plugin
tns run ios --bundle

(nice thing is that changes in .js are applied on the fly)

But with current devxoul/Toaster:master branch, we get:

JS ERROR Error: No initializer found that matches constructor invocation.

[EddyVerbruggen]

@Coeur If that's undefined, it means the Pod isn't actually loaded. Your steps seem OK though so I'm curious. Can you share your project? Otherwise I'll have to retrace your steps and I may end up doing it slightly differently.

[Coeur]

I've fixed the issue by using your fork, like that: https://github.com/Coeur/nativescript-toaster/commit/1e6d422d3902a4c1ed4e590ce625b78fe98484c5
I guess that those missing @objc were the issue...

[edit]
I'll continue looking at the PR tomorrow: got to go for now.

[EddyVerbruggen]

@Coeur The missing @objc's are one thing that I had to change, but to fix the issue this PR tries to address this change was needed as well.

So to reiterate, showing a Toast works, but if you would use this method the app would crash. If you did not show a Toast beforehand, the app would not crash.

[Coeur]

Let's address the Objective-C compatibility in a pull request of its own: #147.
Once done, we can come back here to that issue with setAlternateIconName in NativeScript.

[Coeur]

@EddyVerbruggen I've added a call to setAlternateIconName without triggering any crash: https://github.com/Coeur/nativescript-toaster/commit/0c4ab8196e98fe71f494a817eb6a7e04f19d3084
But I wasn't successful at actually setting the AppIcon-2 resource in a NativeScript project, so to be honest, setting the alternateIconName isn't currently working in my hands.

Could you provide a sample MCVE of the crash issue? Maybe a simple fork of https://github.com/Coeur/nativescript-toaster/ ?

[Coeur]

@EddyVerbruggen As I can't reproduce the issue that you're facing, I'll close this PR for now.
If you get an MCVE (see my previous comment), then we can reopen it. Thank you.