Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do we need a plugin architecture and what does it look like? #1489

Closed
MadeByMike opened this issue Aug 13, 2019 · 7 comments
Closed

Do we need a plugin architecture and what does it look like? #1489

MadeByMike opened this issue Aug 13, 2019 · 7 comments
Assignees

Comments

@MadeByMike
Copy link
Contributor

It seems there is a need for some kind of plugin architecture. Sometimes this can be achieved via hooks and HOC however if there are many popular 'plugins' wrapping many HOCs is not a fantastic API.

I've created this issue to discuss.

Some examples that might be 'plugins' include:

I'm very pro plugins as I think they help find limitations in the core product. An example of this is orderable\sortable fields. To support this I think we need to make some changes to the core platform and if other plugin ideas surface requirements that would be great!

I'd love to see 'plugins' stay really simple. essentially a better API for HOCs something like:

keystone.createList('User', Users, 
{ 
plugins: [
  withTimestamps(timestampPluginOptions), 
  withRoles(rolePluginOptions), 
  ]
}
);

I think plugins should be able to add fields, add hooks and register their own hooks.

@timleslie
Copy link
Contributor

List Plugin API Proposal

In this comment I'll outline a proposal for adding list plugins as a mechanism for adding functionality to a List.

A new field will be added to the config object consumed by createList called plugins, which takes a list of functions with the signature config => config. Each of these functions will have an opportunity to take the config object and return either a new config or a modified config.

The plugin function can make changes such as adding new fields or hooks, adding new options to existing fields (e.g. adding access control), or modifying existing fields.

A trivial example would be

keystone.createList('User', {
  fields: { ... },
  hooks: { ... },
  plugins: [config => ({...config, fields: { ...config.fields, newField: { type: Text })],
}

createList would apply each of the plugins in the order specified, allowing for chaining of different plugins.

The following two pieces of code would be equivalent.

keystone.createList('User', {..., plugins: [plugin1, plugin2] })
keystone.createList('User', plugin2(plugin1({...})))

In this sense, the plugins option is simply a convenience wrapper around what is already possible. The advantage is that it gives us a well defined pattern which people can follow. It also allows the plugin list to be defined programatically.

@gautamsi
Copy link
Member

The plugin function can make changes such as adding new fields or hooks

@timleslie on Hooks, is this possible to modify existing hooks config item to also accept array and run them async as part of this PR? (we discussed over slack). instead of plugin wrap the field hook in async they can push to hooks like 'field.hooks.push(async () => {})`

@timleslie
Copy link
Contributor

Changing the hooks API is out of scope for this API. It's definitely still on the radar though, I think the appropriate pattern will land once we get this plugin API taken care of and a few different types of plugins start to come in.

@gautamsi
Copy link
Member

makes sense.

for the help of community with plugins, I strongly feel that we should delay any initialization of field or list from config until the call to connect is made (or split connect in initialize and connect - so that we can target code which can access uninitialized keystone as well as initialized but not connected kesytone)

@timleslie
Copy link
Contributor

makes sense.

for the help of community with plugins, I strongly feel that we should delay any initialization of field or list from config until the call to connect is made (or split connect in initialize and connect - so that we can target code which can access uninitialized keystone as well as initialized but not connected kesytone)

I'm not sure I understand the desire for this. Can you give an example of a use case where it would be useful to do something "uninitialized" state?

@gautamsi
Copy link
Member

gautamsi commented Aug 16, 2019

@timleslie I have taken another look. One of the use case is to add fields later by things like Auth Strategies, right now if you want to capture social auth username into User collection, You have to do it manually in schema. If we delay list.initFields or introduce another method like List.addField we can push new fields even after the keystone.createList is called.
this scenario is still not possible by list plugins.

edit: created another issues #1498 for this

@stale
Copy link

stale bot commented Feb 15, 2020

It looks like there hasn't been any activity here in over 6 months. Sorry about that! We've flagged this issue for special attention. It wil be manually reviewed by maintainers, not automatically closed. If you have any additional information please leave us a comment. It really helps! Thank you for you contribution. :)

@stale stale bot added the needs-review label Feb 15, 2020
@stale stale bot closed this as completed Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants