fix(cordova-plugin): add check if method exists

[AmitMY]

For: #1327

I get an error, originating there, and I need to know how to handle it

[ihadeed]

See https://github.com/driftyco/ionic-native/blob/master/src/%40ionic-native/core/plugin.ts#L35

[AmitMY]

Thanks @ihadeed , I did not notice it.
However, that undefined check you linked to worries me.. undefined is not triple equal to an undefined string (undefined !== 'undefined')

So I am guessing that check is not good enough.

Also, cordova_not_available and plugin_not_installed both are not correct responses for this situation. if the plugin has an instance, but does not have a method, it should say something like method_not_available and print an error for the correct plugin (in console).

Do you agree?

[AmitMY]

@ihadeed I managed to figure out that the plugin is the Rollbar native plugin.
This check is mandatory..

[ihadeed]

Ah I see what's wrong. I forgot to add typeof in this line: https://github.com/driftyco/ionic-native/blob/master/src/%40ionic-native/core/plugin.ts#L35

It should be:

if (!pluginInstance || (!!methodName && typeof pluginInstance[methodName] === 'undefined')) {

Ideally all checks should be in that method and not in the one you modified. Could you please edit your PR and modify the checkAvailability function instead?

[AmitMY]

@ihadeed Done.
The error message will be wrong though.. It will say plugin_not_installed instead of method_not_available, and will not supply any info on what method is undefined

[ihadeed]

@AmitMY

Thanks!

You're right, the message will be wrong. But I think it's good for most cases. Usually all the methods that we have in our wrappers exist on the original plugin. Which means that methodName should always exist.

---

Here is why we check both methodName and pluginRef and in both cases we assume that the plugin is not installed/available:

If you look at the screen orientation plugin for example, it used to add lockOrientation property to the window.screen object. In this case if we check window.screen only, it will exist on all browsers, but lockOrientation will not exists. So in this case the right error message should say "plugin not installed" or "cordova not available".

---

If you see any plugins that might benefit from having different error messages for pluginRef and methodName, then let me know and we'll have a look at them. I don't mind having more detailed errors, but it's always good to minimize the code that we don't need. (specially run-time stuff like this)

---

P.S: just fixed the pluginRef for the Rollbar plugin: f396940

[AmitMY]

@ihadeed Thanks a lot!
I guess that was the issue with the rollbar plugin.

Its funny, I have rollbar.js also, which means I have around 200 errors, 20 unique, giving me an error, because rollbar native has a wrong reference :P

I find it hilarious.

[AmitMY]

Any chance for a new release? this is quite a major fix (for my code at least)