search

ratson / cordova-plugin-admob-free

New development has been moved to "admob-plus-cordova", https://github.com/admob-plus/admob-plus/tree/master/packages/cordova
https://github.com/admob-plus/admob-plus
MIT License
494 stars 210 forks source link

change js-module clobbers #48

Closed becvert closed 7 years ago

becvert commented 7 years ago

instead of window.admob, I suggest using cordova.plugins.admob

<clobbers target="cordova.plugins.admob" />

I feel like not enough plugins use cordova.plugins as a placeholder. it's a shame.

what do you think?

ratson commented 7 years ago

I am not sure if there is any advantage for using cordova.plugins.admob, and it seems too long for typing. Could you point me to any reference that is recommending it?

becvert commented 7 years ago

I don't think there's actually any recommandation from cordova. I've just seen that in a few plugins. But it's a smart move as one best practice in JavaScript is to avoid cluttering the global namespace. I don't know if that's a big advantage but then I know where the cordova plugins are, and in DevTools I can use method autocompletion for instance. And it isn't too long and I always create a var in that case, eg. var admob = cordova.plugins.admob; if I need to. I put my own plugins under this placeholder, it's a personal preference in the end. That's was just a suggestion. Feel free to close or wait for input from other devs. Thanks

vintage commented 7 years ago

I'm good with the global admob variable (even if using it once and exposing it using angular2 service). I see no gain in changing this.