wrap-cli icon indicating copy to clipboard operation
wrap-cli copied to clipboard

Allow user to get plugin config from client

Open pileks opened this issue 3 years ago • 3 comments

Closes #839 The implementation fetches the plugin config by calling its factory function within the PluginPackage and extracting the config from the resulting object.

pileks avatar Aug 11 '22 13:08 pileks

It seems expensive to construct a plugin instance to get the config. What are the pros and cons of changing the PluginPackage type to include a pointer to the config?

export type PluginPackage<TConfig> = {
  factory: () => PluginModule<TConfig>;
  manifest: PluginPackageManifest;
  config: TConfig;
};

krisbitney avatar Aug 15 '22 16:08 krisbitney

@krisbitney The main con that came to mind when looking at this issue was that we would need to trust the developer that their implementation of the PluginFactory for their plugin actually assigns the config being used by the plugin to that field (or assigns it at all). This does sound like something we could enforce though, so it could be considered a moot point. 😄

My main issue in general here has to do with the recent work of moving away from plugin configs and placing any configuration into Env. I have actually recently worked on an issue that's related to this: https://github.com/polywrap/monorepo/pull/1060

The question of "how much time we want to spend on this?" kind of comes to mind, and whether we are really moving away from configs into envs, making even my implementation obsolete at one point in the future. And in any case, do we foresee that we'll be using this getConfig often enough to warrant us making these significant changes in order to be able to fetch the config of a plugin.

I'm sure that I'm missing something important when considering all of this and I'd love to chat regarding this so feel free to hit me up on Discord and we'll come up with something smart!

pileks avatar Aug 16 '22 21:08 pileks

All of that makes sense. Thanks!

krisbitney avatar Aug 17 '22 07:08 krisbitney

I don't think this feature makes sense, since we are trying to phase out plugins from the client Even if it did, why not do client.getPlugin() then get the config from there?

On your point of moving from configs to envs, that's not completely true, configs can do what envs can not (e.g. pass objects by reference) so they will always have their place.

nerfZael avatar Aug 18 '22 11:08 nerfZael

I agree with Jure, I think we should instead just expose a helper client.getPlugin(uri) method.

dOrgJelli avatar Aug 31 '22 15:08 dOrgJelli