feat(cli): locate plugins by allowlist

[imhoffd]

TODO:

• native updates to handle getting plugin config from plugins or pluginsConfig (depends on feat(iOS): Refactoring configuration #3759 and feat(Android): Refactoring configuration #3778)
• send a warning in the Capacitor CLI for using plugins as an object

Intended usage:

const config: CapacitorConfig = {
  appId: 'com.capacitorjs.testapp',
  appName: 'capacitor-testapp',
  bundledWebRuntime: false,
  webDir: 'build',
  android: {
    includePlugins: [
      ...
    ],
  },
  ios: {
    includePlugins: [
      ...
    ],
  },
  includePlugins: [
    '@capacitor/action-sheet',
    '@capacitor/app',
    '@capacitor/app-launcher',
    '@capacitor/browser',
    '@capacitor/clipboard',
    '@capacitor/device',
    '@capacitor/dialog',
    '@capacitor/haptics',
    '@capacitor/keyboard',
    '@capacitor/motion',
    '@capacitor/network',
    '@capacitor/screen-reader',
    '@capacitor/share',
    '@capacitor/status-bar',
    '@capacitor/storage',
    '@capacitor/text-zoom',
    '@capacitor/toast',
  ],
  plugins: {
    SplashScreen: {
      launchShowDuration: 0
    }
  },
};

If includePlugins is not specified, then the old way is used: it looks at package.json for potential plugins.

depends on #3756
depends on #3759
depends on #3778
resolves #3265

[aparajita]

We could make getConfigValue on iOS work for plugins or pluginsConfig and then I don't know how Android would work

Why wouldn't that work on Android?

[imhoffd]

Why wouldn't that work on Android?

Because we don't have something like it. Some plugins access the config file directly. We could add it to v3, I suppose, but I haven't thought of the implications yet.

[aparajita]

Are you saying getConfigValue no longer exists in v3 currently?

[imhoffd]

No, I'm saying it never existed for Android.

[imhoffd]

What I mean is our plugins (and likely other plugins) aren't required to use getConfigValue (and some don't). They can access the config directly. We are trying to improve the Plugin API for v3 (#3678 for example)

[aparajita]

No, I'm saying it never existed for Android.

🤔 What's this then?

capacitor/android/capacitor/src/main/java/com/getcapacitor/Plugin.java

Line 144 in 249073d

public Object getConfigValue(String key) {

[aparajita]

I'm definitely all for improving the config API! I have some improvements to contribute.

[imhoffd]

🤔 What's this then?

It's not used or documented anywhere. 😂 But it should be. Then we can do the pluginsConfig/plugins logic in there.

[aparajita]

Yes, there's a bunch of config code that is not documented... and should have been. 😁

I added API to make two things easier:

• Get a value from a plugin call and fall back to plugin config.

• Allow platform-specific options to be put in a sub-object. So you have a choice of doing this:

plugins: {
  SplashScreen: {
    iosSpinnerStyle: 'small',
    iosImageDisplayMode: 'aspectFill',
    androidSpinnerStyle: 'inverse',
    androidImageDisplayMode: 'aspectFill'
  }
}

or this:

plugins: {
  SplashScreen: {
    ios: {
      spinnerStyle: 'small',
      imageDisplayMode: 'aspectFill'
    },
    android: {
      spinnerStyle: 'inverse',
      imageDisplayMode: 'aspectFill'
    }
  }
}

You can pass those options to a plugin call in either way as well.

[aparajita]

Don't remove all of the "unused" config code, I'm using most of it.

[jcesarmobile]

I don't see this feature, so users would have to manually handle the plugin list every time they npm install or npm uninstall a plugin? We are working on making the android import automatic so users don't have to do it, but then we make this new thing manual?

By the look of the linked issue, it doesn't look like they know which plugins they need on their build pipeline, I don't think this is what they asked for.

Also, I wouldn't replace the plugins object to use it as the list, all apps have that object, so all apps would need to be changed, if we go with this, I think it should be better to leave plugins alone and put the list on pluginList or something. But again, I don't see this feature nor think this is what the users who proposed this were thinking about.

[imhoffd]

so users would have to manually handle the plugin list every time they npm install or npm uninstall a plugin?

Only if they choose to. We've seen that the "automatic" way of getting this list isn't good enough for some people. This feature would allow people who have many plugins installed to only include some. It would also enable people to include plugins that are dependencies of other packages (not in their package.json).

By the look of the linked issue, it doesn't look like they know which plugins they need on their build pipeline, I don't think this is what they asked for.

If you're referring to the "app shell" approach here, I think the JS app must know which plugins they need because ultimately they're using them, even if they're transitive dependencies.

This is probably not what they asked for, but the changes in this PR will enable their use case and many others. For their approach, they could do something like this. Or they could just pull in all the dependencies from each JS component's package.json files. The Capacitor CLI will still exclude packages that aren't Capacitor plugins from the list.

all apps have that object, so all apps would need to be changed

It's a simple change (just rename "plugins" key to "pluginsConfig"), but I get your point. The Capacitor CLI could do this automatically, but I'm not sure how you feel about that. I'm fine renaming the keys to something else, I just figured we could take this opportunity to have the ideal keys.

[jcesarmobile]

I agree that the automatic way is not good enough in some cases, but I don't think this way makes it better for those cases neither, the problem is still there, a package installs a capacitor plugin as a dependency, it's in node_modules but not in the package.json, so it's not detected.
Two options:

1. They add the plugin to the package.json
2. They add the plugin to this new list.

I see both options equally bad.
Well, second is even worse because they now also have to add the ones in the package.json that were automatically detected before and the ones that weren't detected.

Would be good to have some input from the users who requested/reported this before continuing the work on it.

[aparajita]

If we look at the way most other packages deal with plugins, it's more or less one of two ways:

• Import plugin and then include the import in a list.
• Specify the name in a list and try to resolve the plugin using naming conventions.

Of the two, the second is preferable.

But, there's a better and completely infallible way to generate the plugin list automatically:

• Hook into the webpack parser.
• Whenever you see a class that extends core.WebPlugin, wait until the constructor is parsed, then look for the name passed to super.

Not easy, but very doable.

[imhoffd]

I see both options equally bad.

The difference is with a dynamic configuration file, the list can come from anywhere. The users who originally reported this don't know the list of plugins to include during npx cap sync, but they can get that list by reading a file in the JS component or a number of other ways.

[imhoffd]

But, there's a better and completely infallible way to generate the plugin list automatically

We don't want to assume people are using webpack. Our solutions must be general enough to work for everyone.

[aparajita]

We don't want to assume people are using webpack. Our solutions must be general enough to work for everyone.

True.

[jcesarmobile]

Ok, if you think people can get the list programmatically and use the dynamic configuration file feel free to merge.

But I think you shouldn’t use the plugins object for this.
And also think the list should allow to filter plugins per platform, I’ve seen a lot of requests for cordova and a few for capacitor for being able to install a plugin only for a platform but not the other even if the plugin supports it.

[aparajita]

And also think the list should allow to filter plugins per platform

Per-platform plugin prefs are already supported in the config file, so it follows the plugin list itself might as well allow the same.

[imhoffd]

But I think you shouldn’t use the plugins object for this.

done!

And also think the list should allow to filter plugins per platform

done!