apache / cordova-lib

Apache Cordova Tooling Library
https://cordova.apache.org/
Apache License 2.0
221 stars 242 forks source link

Adding plugins for only specific platforms #892

Open ardabeyazoglu opened 2 years ago

ardabeyazoglu commented 2 years ago

Feature Request

I want to add some plugins to only single platform or skip for a certain platform. I already found a way to implement this, and I am willing to develop this feature and send a pull request if approved.

Motivation Behind Feature

Multi platform apps do not always require same functionality. For example, I may have a client who needs nfc features and use only android devices, whereas ios clients don't need them. Besides, one may prefer not to add some specific functionality to each platform although device supports it. So why not just add the plugin and don't implement the feature ? Short answer: to avoid unnecessary approval trouble. For example; If I add phonegap-nfc plugin, the apple approval team see that I am using nfc and ask me to prove how I use it and why I use it ? It is very hard to show what you don't implement and the apple team will not approve it until they see a convincing explanation / proof. The issue becomes especially important when developing ad-hoc apps for customers instead of product apps that are usually same across platforms.

Feature Description

It can be implemented using cli variables that are already built in. A global variable, non conflicting variable such as "cordova-platform" can be used:

cordova plugin add some-cordova-plugin --variable cordova-platform=android cordova plugin add some-cordova-plugin --variable cordova-platform=android,browser

The command skips install steps for unspecified platforms and save preference to config.xml, so that when a platform is added/removed it can repeat the same behaviour.

Alternatives or Workarounds

It is impossible to do it without manual intervention right now. The only way is to add plugin and then delete specific folders and files which is quite cumbersome and highly repetitive.

breautek commented 2 years ago

Personally I'd support this kind of feature, but I don't know if overloading the --variable is a good idea for this. I think it may be best to keep that reserved for actual plugin variables.

I think it may better to introduce a new --platforms argument. But I do understand the motivation of reusing the --variable flag, as it would allow restoration to work easily because it would be compatible with the current package.json structure that cordova uses.

To solve the restoration issue, the --platforms argument will require a more extensible cordova structure in it's package.json. Which I think is doable but would likely require a series of PRs before a PR can be made to tackle this issue. This obviously requires a bit extra work but I think is worth it for a cleaner API. It's always easy to add something, but difficult to remove or change.

Interested in hearing your thoughts on this.

It is impossible to do it without manual intervention right now. The only way is to add plugin and then delete specific folders and files which is quite cumbersome and highly repetitive.

Just to add another work around. The plugin could always be forked and have it's platform implementation removed (even just from it's plugin.xml entry) and that should be sufficient to use a plugin and omit one platform's implementation. The drawback of course is that you'd have to manage the fork and periodically pull in updates from upstream and deal with merge conflicts.

ardabeyazoglu commented 2 years ago

Hi @breautek , thanks for the answer.

Just to add another work around. The plugin could always be forked and have it's platform implementation removed (even just from it's plugin.xml entry) and that should be sufficient to use a plugin and omit one platform's implementation. The drawback of course is that you'd have to manage the fork and periodically pull in updates from upstream and deal with merge conflicts. Seems like a better monkey patch than mine, but it is still not very convenient as you explained.

Adding a new "--platform" config would indeed be better engineering, however I don't have enough free time and low-level cordova knowledge to implement this. Not even sure about my own proposal, but I had some progress there. Also, --platform would require a breaking change in package.json layout I think.

breautek commented 2 years ago

Also, --platform would require a breaking change in package.json layout

It would, but I think it can be implemented in a way that we can migrate a structure to a more extensible structure pretty easily. I'd have to check with some other PMC members first though.

however I don't have enough free time and low-level cordova knowledge to implement this.

That's okay. Apache is no obligation, 100% volunteers. This would likely be an iterative change anyway. E.g. a few PRs would have to be made before this issue can be tackled. That's assuming the new --platform route is taken of course.

ardabeyazoglu commented 2 years ago

I have another idea to implement it with a hook which would be much simpler and still automated, however there is no hook for that. Simply an "after_plugin_fetch" hook could modify the plugin.xml and remove the undesired platforms and that's it. It is also restorable. No need to clone the plugin or deal with merge conflicts. While this solution would be up to the user to implement with a hook, it is straightforward. In this case, we just need to add a few lines of code to cordova/plugin/add.js to support the hook.

Also, I got another idea on implementing a built-in feature without using --platform or --variable as well. For example; suppose that config.xml might have an element that contains blacklist of certain plugin ids for specific platforms. plugin add and platform add check this section before calling plugman.install and skip for blacklisted plugins.

breautek commented 2 years ago

There is a number of hooks. That's a good idea, that's probably the most elegant way of working around this issue.

Would the existing after_plugin_add or after_plugin_install do the trick? Perhaps even before_prepare ?

https://cordova.apache.org/docs/en/11.x/guide/appdev/hooks/index.html

ardabeyazoglu commented 2 years ago

They don't help here because plugins are already installed to the platform when they are called.

cklam commented 2 years ago

Sent from Yahoo Mail for iPhone

On Thursday, May 5, 2022, 12:39 PM, Arda Beyazoğlu @.***> wrote:

They don't help here because plugins are already installed to the platform when they are called.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

NikitaKA commented 8 months ago

Still need this in 2024. For example: onesignal-cordova-plugin (version 5). Cordova installs every plugin for every platform (ios, android and browser for example). But onesignal-cordova-plugin does not support the browser platform, to turn on push notifications in PWA you need to use Service Worker from OneSignal's Web SDK, not plugin. Unfortunately, Service Worker interferes with the onesignal-cordova-plugin and throws exceptions.

The only way I found how to fix that is to manually edit the /platforms/browser/platform_www/cordova_plugins.js file and remove everything linked to cordova-onesignal-plugin. It looks fine to do that once for a one project, but I have about 100 different projects and this approach is not acceptable.

I think using hooks is a good idea for a workaround, but ignoring the unsupportable platforms during plugin installation looks a way better approach.

breautek commented 8 months ago

Still need this in 2024. For example: onesignal-cordova-plugin (version 5). Cordova installs every plugin for every platform (ios, android and browser for example). Unfortunatelly, onesignal-cordova-plugin does not support browser platform, to turn on push notifications in PWA you need to use Service Worker from OneSignal's Web SDK, not plugin. But Service Worker interfere with onesignal-cordova-plugin and throws exceptions. 😐

The only way I found how to fix that is to manually edit /platforms/browser/platform_www/cordova_plugins.js file and remove everything linked to cordova-onesignal-plugin. It looks fine to do that once for one project, but I have about 100 different projects and this approach not acceptable.

In onesignal's case, they are providing a JS module for all platforms (because it's not isolated inside a <platform> block. If onesignal uses an alternate library for the browser that isn't a cordova plugin, then they should be able to move https://github.com/OneSignal/OneSignal-Cordova-SDK/blob/4ed52df056f75b75276c3a8997e574d2a0f900b9/plugin.xml#L16-L18 into the <platform> block, but they'll need to have 2 copies of it, one for iOS and one for android. This should prevent the JS module from being installed for browser targets, but have it installed for iOS and android targets. Again suggesting this more as a workaround, but if one signal's plugin doesn't support browser, it might be better for one signal to adjust their config so it won't be installed on browser targets to begin with.

NikitaKA commented 8 months ago

Still need this in 2024. For example: onesignal-cordova-plugin (version 5). Cordova installs every plugin for every platform (ios, android and browser for example). Unfortunatelly, onesignal-cordova-plugin does not support browser platform, to turn on push notifications in PWA you need to use Service Worker from OneSignal's Web SDK, not plugin. But Service Worker interfere with onesignal-cordova-plugin and throws exceptions. 😐 The only way I found how to fix that is to manually edit /platforms/browser/platform_www/cordova_plugins.js file and remove everything linked to cordova-onesignal-plugin. It looks fine to do that once for one project, but I have about 100 different projects and this approach not acceptable.

In onesignal's case, they are providing a JS module for all platforms (because it's not isolated inside a <platform> block. If onesignal uses an alternate library for the browser that isn't a cordova plugin, then they should be able to move https://github.com/OneSignal/OneSignal-Cordova-SDK/blob/4ed52df056f75b75276c3a8997e574d2a0f900b9/plugin.xml#L16-L18 into the <platform> block, but they'll need to have 2 copies of it, one for iOS and one for android. This should prevent the JS module from being installed for browser targets, but have it installed for iOS and android targets. Again suggesting this more as a workaround, but if one signal's plugin doesn't support browser, it might be better for one signal to adjust their config so it won't be installed on browser targets to begin with.

Thank you for quick response! I will create an issue in plugin repo.