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

Proposal: Plugin Architecture Refactor #769

Closed
jgable opened this Issue Sep 16, 2013 · 35 comments

Comments

Projects
None yet
10 participants
@jgable
Member

jgable commented Sep 16, 2013

While trying to put together a non trivial plugin that required adding routes, assets and filters I ran into some road blocks. Here are my notes aggregated up and made into an epic of sorts.

Asynchronous Filter Changes

  1. ghost.doFilter should allow filters to return promises, and be refactored to return a promise itself to match the rest of our code.
  2. ghost.registerAsyncThemeHelper should allow registering async theme helpers along with the existing sync ones so that helpers who use ghost.doFilter will still work.
  3. Refactor core theme helpers to be isolated from handlebars so we can test them in isolation. Basically just move the functions passed to ghost.registerThemeHelper onto something like coreHelpers.author and reference it that way. Then, update the tests to not rely on handlebars rendering synchronously.
  4. Pass the server to initPlugins so plugins can register routes and file assets.

Misc. Nice to Haves

  • Provide a filter for when a post is created/saved/updated; use case is that I wanted to add the default kudos value for each post in my new posts_kudos table but instead had to check on each prePostsRender filter whether the row existed already.
  • Provide some way to get the current post id from the client side code for a plugin.
  • Creating tables and data access through knex is kind of wonky, is there a better way?
  • Make it easier to check if a script is already added (like jquery or underscore) in ghost_foot filter.

Feel free to add to this / modify.

I have 1-3 mostly done but will need to rebase on latest, I'll start moving my code from previous PR's into a new branch for this.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 27, 2013

Member

@jgable Soo.. much time later :)

It's funny, I'd never really imagined plugins requiring knex directly. Of course it's possible and you can add all kinds of wonder, but your question of is there a better way has to be almost definitely?

There was the consideration of having a custom data table which was many-to-many mapped to posts, so that plugins who wanted to store custom values for posts, such as your kudos, had a simple way to do it. At the same time, you didn't really need many-to-many...

With regards to all the first items around promises etc, you had these done and then closed the PRs, did you want to reopen them? I didn't even get chance to look!

Member

ErisDS commented Sep 27, 2013

@jgable Soo.. much time later :)

It's funny, I'd never really imagined plugins requiring knex directly. Of course it's possible and you can add all kinds of wonder, but your question of is there a better way has to be almost definitely?

There was the consideration of having a custom data table which was many-to-many mapped to posts, so that plugins who wanted to store custom values for posts, such as your kudos, had a simple way to do it. At the same time, you didn't really need many-to-many...

With regards to all the first items around promises etc, you had these done and then closed the PRs, did you want to reopen them? I didn't even get chance to look!

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Sep 27, 2013

Member

Right, just so we're on the same page about where this is all coming from I put the code for my example Kudos Plugin up in a gist.

I could deal with having just a custom data table for posts, especially for just v1.0 of this stuff. We could make some helper functions in the plugin base class to help with the common things like creating and ensuring the value is there on post creation and what-not.

Do you want to add an item to this to create that custom data table again and maybe put an api around it?

As for the other promise stuff, I'll wrap them up again in new PR's since we've had a lot of activity lately that may have affected things. I've still got them in their own branches so it shouldn't be that hard.

Member

jgable commented Sep 27, 2013

Right, just so we're on the same page about where this is all coming from I put the code for my example Kudos Plugin up in a gist.

I could deal with having just a custom data table for posts, especially for just v1.0 of this stuff. We could make some helper functions in the plugin base class to help with the common things like creating and ensuring the value is there on post creation and what-not.

Do you want to add an item to this to create that custom data table again and maybe put an api around it?

As for the other promise stuff, I'll wrap them up again in new PR's since we've had a lot of activity lately that may have affected things. I've still got them in their own branches so it shouldn't be that hard.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Sep 29, 2013

Member

Thanks for the PR, apologies for being a bit slow about them atm - I have my brain in other places.

Thanks also for providing your code for the Kudos plugin, it's great to see a real-world example of what a plugin might actually look like!

I think my first, biggest question when I saw that was:

How can we make this line of code sexier?

GhostPlugin = require('../../../core/server/plugins/GhostPlugin'),

I'm having a little play with creating some plugins myself atm, so will have a good play with your PR over the next few days. I feel like we need to have a few examples to make informed decisions.

Member

ErisDS commented Sep 29, 2013

Thanks for the PR, apologies for being a bit slow about them atm - I have my brain in other places.

Thanks also for providing your code for the Kudos plugin, it's great to see a real-world example of what a plugin might actually look like!

I think my first, biggest question when I saw that was:

How can we make this line of code sexier?

GhostPlugin = require('../../../core/server/plugins/GhostPlugin'),

I'm having a little play with creating some plugins myself atm, so will have a good play with your PR over the next few days. I feel like we need to have a few examples to make informed decisions.

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Sep 29, 2013

Member

I think we could make the GhostPlugin class it's own module, eg
ghost-plugin. That would also allow us to make different implementations
(RouterPlugin, DataPlugin) and keep them separate from the ghost code base.

Member

jgable commented Sep 29, 2013

I think we could make the GhostPlugin class it's own module, eg
ghost-plugin. That would also allow us to make different implementations
(RouterPlugin, DataPlugin) and keep them separate from the ghost code base.

@jamesbloomer

This comment has been minimized.

Show comment
Hide comment
@jamesbloomer

jamesbloomer Sep 30, 2013

Member

Surely a simple require is sexy enough? :-)

Member

jamesbloomer commented Sep 30, 2013

Surely a simple require is sexy enough? :-)

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Oct 16, 2013

Member

I have been writing some more stuff down. I'm just gonna brain dump it all in here for now and get feedback. The original topic still stands for the most part, and we can update it as necessary.

  • Post Based Plugins
    • View Post data
      • Solution: Filters should handle this
    • Add custom data related to posts (eg. KudosVal)
      • Solution: Custom post meta data table/api proposed by Hannah
    • Register client side scripts
      • Solution: Can currently use the ghost_head or ghost_foot filter. But, we should expose a registerScript method, or refactor the ghost_* filters to have a convenience method for checking whether a script has already been registered.
    • Need to get access to post id from the JavaScript
      • Solution?: Ghost core should put a client side script on every page using the ghost_foot filter.
  • Dashboard plugins
    • Client scripts
      • Solution above. Will also probably need some core/server/client extra work
    • Client assets (images, fonts)
      • Solution: Register static asset directory with express.static (maybe make this easier, through a function on the GhostPlugin class)
  • Router Based Plugins
    • API routes
      • Solution: use ghost.server that is passed to plugin activate method.
    • View routes (render a new view with the themes default layout, or its own)
      • e.g. myblog.com/colophon
      • Not sure, we can't really register another views root in addition to the themes
  • Plugins may need configuration/settings
    • Could start with just config files in their root
    • Will need a way to render these on settings page eventually
      • Solution: New filters for settings pages that would let plugins expose their settings
  • Core Changing Plugins
    • OpenID Login
      • Not sure, security/login is handled by admin view. Not very extensible at the moment.
    • File Storage
      • S3
        • Solution: Work has already begun on abstracting file storage. We could make it so that you could do ghost.files = new S3FileProvider() or something on plugin activate.
      • Azure
    • Editor Based Plugins
      • Code highlighting?
        • Solution: These are going to require some filters on the admin side, both server and client side will have to modified to work for these. On the server side, more filters live postSaving, postCreated, postUpdated could help. On the client side, we'll need to expose a way for plugins to be called on each update, or on special occasions that they can create.
          • A plugin might register for markdown content changes and regex match a code fence, then in the preview pane apply the styles to highlight the code.
      • Reading time?
      • Placehold.it?
      • Random image search from flickr
    • Security things?
      • Not sure what this could even mean but someone mentioned it in the chat room.
Member

jgable commented Oct 16, 2013

I have been writing some more stuff down. I'm just gonna brain dump it all in here for now and get feedback. The original topic still stands for the most part, and we can update it as necessary.

  • Post Based Plugins
    • View Post data
      • Solution: Filters should handle this
    • Add custom data related to posts (eg. KudosVal)
      • Solution: Custom post meta data table/api proposed by Hannah
    • Register client side scripts
      • Solution: Can currently use the ghost_head or ghost_foot filter. But, we should expose a registerScript method, or refactor the ghost_* filters to have a convenience method for checking whether a script has already been registered.
    • Need to get access to post id from the JavaScript
      • Solution?: Ghost core should put a client side script on every page using the ghost_foot filter.
  • Dashboard plugins
    • Client scripts
      • Solution above. Will also probably need some core/server/client extra work
    • Client assets (images, fonts)
      • Solution: Register static asset directory with express.static (maybe make this easier, through a function on the GhostPlugin class)
  • Router Based Plugins
    • API routes
      • Solution: use ghost.server that is passed to plugin activate method.
    • View routes (render a new view with the themes default layout, or its own)
      • e.g. myblog.com/colophon
      • Not sure, we can't really register another views root in addition to the themes
  • Plugins may need configuration/settings
    • Could start with just config files in their root
    • Will need a way to render these on settings page eventually
      • Solution: New filters for settings pages that would let plugins expose their settings
  • Core Changing Plugins
    • OpenID Login
      • Not sure, security/login is handled by admin view. Not very extensible at the moment.
    • File Storage
      • S3
        • Solution: Work has already begun on abstracting file storage. We could make it so that you could do ghost.files = new S3FileProvider() or something on plugin activate.
      • Azure
    • Editor Based Plugins
      • Code highlighting?
        • Solution: These are going to require some filters on the admin side, both server and client side will have to modified to work for these. On the server side, more filters live postSaving, postCreated, postUpdated could help. On the client side, we'll need to expose a way for plugins to be called on each update, or on special occasions that they can create.
          • A plugin might register for markdown content changes and regex match a code fence, then in the preview pane apply the styles to highlight the code.
      • Reading time?
      • Placehold.it?
      • Random image search from flickr
    • Security things?
      • Not sure what this could even mean but someone mentioned it in the chat room.
@jamesbloomer

This comment has been minimized.

Show comment
Hide comment
@jamesbloomer

jamesbloomer Oct 17, 2013

Member

I've had a bit of time to do a bit more with the file storage (tidying up, thinking about S3 etc.). I'll try and get it into a fit state and on the image-refactor branch. Essentially though they're just different modules with the same interface.

Member

jamesbloomer commented Oct 17, 2013

I've had a bit of time to do a bit more with the file storage (tidying up, thinking about S3 etc.). I'll try and get it into a fit state and on the image-refactor branch. Essentially though they're just different modules with the same interface.

@thehydroimpulse

This comment has been minimized.

Show comment
Hide comment
@thehydroimpulse

thehydroimpulse Oct 20, 2013

I'd recommend doing it like @jgable suggested (seperate repo for the plugin). That's what we've been doing with Tower.js. It makes things much more testable, leaner, and awesome to require. Then, you don't have to worry about the path issues.

thehydroimpulse commented Oct 20, 2013

I'd recommend doing it like @jgable suggested (seperate repo for the plugin). That's what we've been doing with Tower.js. It makes things much more testable, leaner, and awesome to require. Then, you don't have to worry about the path issues.

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Oct 22, 2013

Member

I've been getting a little antsy about this, so I put together a repo of what I think the base plugin classes could look like over at jgable/ghost-plugin. We can transfer repo ownership and all that later if you'd like.

Base Plugin Classes

Then, I have one big convenience base class that should prove really helpful; PostPlugin. It combines all the rest and should handle a lot of use cases.

Here, for instance, is what a Disqus Plugin might look like:

var fs = require('fs'),
    path = require('path'),
    _ = require('lodash'),
    when = require('when'),
    GhostPlugins = require('ghost-plugin'),
    PostPlugin = GhostPlugins.PostPlugin,
    DisqusPlugin;

DisqusPlugin = PostPlugin.extend({
    filters: {
        "prePostsRender": "addDisqusToPost"
    },

    initialize: function () {
        _.bindAll(this, 'addDisqusToPost');

        // Create a template of the footer content
        var templateContents = fs.readFileSync(path.join(__dirname, 'disqus-post.tpl')).toString();
        this.postFooterTemplate = _.template(templateContents);
    },

    addDisqusToPost: function (post) {
        if (_.isArray(post)) {
            // Don't add to listing
            return post;
        }

        post.html += this.postFooterTemplate({
            // TODO: Load this from ghost instance or something
            shortname: "Change Me",
            post: post
        });

        return post;
    }
});

module.exports = DisqusPlugin;

Here is the disqus-post.tpl:

<div id="disqus_thread"></div>
<script type="text/javascript">
    var disqus_shortname = '<%= shortname %>'; // required: replace example with your forum shortname
    var disqus_identifier = '<%= post.id %>'; // make sure to use the post.id as an identifier, otherwise disqus will use the pages url per default, which might be problematic...

    /* * * DON'T EDIT BELOW THIS LINE * * */
    (function() {
        var dsq = document.createElement('script'); dsq.type = 'text/javascript'; dsq.async = true;
        dsq.src = '//' + disqus_shortname + '.disqus.com/embed.js';
        (document.getElementsByTagName('head')[0] || document.getElementsByTagName('body')[0]).appendChild(dsq);
    })();
</script>
<noscript>Please enable JavaScript to view the <a href="http://disqus.com/?ref_noscript">comments powered by Disqus.</a></noscript>
<a href="http://disqus.com" class="dsq-brlink">comments powered by <span class="logo-disqus">Disqus</span></a>

Also, be sure to check out the tests for a little more insight on instantiating the classes and all that.

Member

jgable commented Oct 22, 2013

I've been getting a little antsy about this, so I put together a repo of what I think the base plugin classes could look like over at jgable/ghost-plugin. We can transfer repo ownership and all that later if you'd like.

Base Plugin Classes

Then, I have one big convenience base class that should prove really helpful; PostPlugin. It combines all the rest and should handle a lot of use cases.

Here, for instance, is what a Disqus Plugin might look like:

var fs = require('fs'),
    path = require('path'),
    _ = require('lodash'),
    when = require('when'),
    GhostPlugins = require('ghost-plugin'),
    PostPlugin = GhostPlugins.PostPlugin,
    DisqusPlugin;

DisqusPlugin = PostPlugin.extend({
    filters: {
        "prePostsRender": "addDisqusToPost"
    },

    initialize: function () {
        _.bindAll(this, 'addDisqusToPost');

        // Create a template of the footer content
        var templateContents = fs.readFileSync(path.join(__dirname, 'disqus-post.tpl')).toString();
        this.postFooterTemplate = _.template(templateContents);
    },

    addDisqusToPost: function (post) {
        if (_.isArray(post)) {
            // Don't add to listing
            return post;
        }

        post.html += this.postFooterTemplate({
            // TODO: Load this from ghost instance or something
            shortname: "Change Me",
            post: post
        });

        return post;
    }
});

module.exports = DisqusPlugin;

Here is the disqus-post.tpl:

<div id="disqus_thread"></div>
<script type="text/javascript">
    var disqus_shortname = '<%= shortname %>'; // required: replace example with your forum shortname
    var disqus_identifier = '<%= post.id %>'; // make sure to use the post.id as an identifier, otherwise disqus will use the pages url per default, which might be problematic...

    /* * * DON'T EDIT BELOW THIS LINE * * */
    (function() {
        var dsq = document.createElement('script'); dsq.type = 'text/javascript'; dsq.async = true;
        dsq.src = '//' + disqus_shortname + '.disqus.com/embed.js';
        (document.getElementsByTagName('head')[0] || document.getElementsByTagName('body')[0]).appendChild(dsq);
    })();
</script>
<noscript>Please enable JavaScript to view the <a href="http://disqus.com/?ref_noscript">comments powered by Disqus.</a></noscript>
<a href="http://disqus.com" class="dsq-brlink">comments powered by <span class="logo-disqus">Disqus</span></a>

Also, be sure to check out the tests for a little more insight on instantiating the classes and all that.

@gotdibbs

This comment has been minimized.

Show comment
Hide comment
@gotdibbs

gotdibbs Oct 23, 2013

Member

@jgable I just gave your stuff a solid pass with my tired eyes. Looks very good for an initial draft!

Just a couple of notes:

  • I took a pass at simplifying the multiple inheritance you were doing in the PostPlugin file. You can see that here: gotdibbs/ghost-plugin@d5da5c5c8e81d090f071cc35af6ddade455eb78f.
  • DatabasePlugin will probably need some significant foresight/security reviews when you consider their ability to add/drop schema. Maybe provide our own wrapper around knex.js?
  • I have a similar security concern around giving plugins direct access to our server/express objects. Perhaps this is something we can wrap as well?
  • (Jumping way ahead) How would you envision we distribute the base classes? Included with Ghost but not needed in individual plugin packages? NPM module? I'm just thinking we might want to provide intellisense files for peoples to develop against and/or mocks for peoples to test with.
Member

gotdibbs commented Oct 23, 2013

@jgable I just gave your stuff a solid pass with my tired eyes. Looks very good for an initial draft!

Just a couple of notes:

  • I took a pass at simplifying the multiple inheritance you were doing in the PostPlugin file. You can see that here: gotdibbs/ghost-plugin@d5da5c5c8e81d090f071cc35af6ddade455eb78f.
  • DatabasePlugin will probably need some significant foresight/security reviews when you consider their ability to add/drop schema. Maybe provide our own wrapper around knex.js?
  • I have a similar security concern around giving plugins direct access to our server/express objects. Perhaps this is something we can wrap as well?
  • (Jumping way ahead) How would you envision we distribute the base classes? Included with Ghost but not needed in individual plugin packages? NPM module? I'm just thinking we might want to provide intellisense files for peoples to develop against and/or mocks for peoples to test with.
@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Oct 23, 2013

Member

I took a pass at simplifying the multiple inheritance you were doing in the PostPlugin file. You can see that here: gotdibbs/ghost-plugin@d5da5c5.

I like it, but I want to call it something like extendMultiple instead of define. I'll incorporate that in.

DatabasePlugin will probably need some significant foresight/security reviews when you consider their ability to add/drop schema. Maybe provide our own wrapper around knex.js?

I agree this is sort of wide open at the moment. Hannah brought up the idea of custom post data, which I like and can be incorporated into this class once it's fleshed out. But, I also like having the power to do some really advanced stuff with direct db access.

I have a similar security concern around giving plugins direct access to our server/express objects. Perhaps this is something we can wrap as well?

Right now we pass the server straight through, but we could make a proxy that exposes only the things we want to (get, post, use, etc). Then, those proxy handlers could also do some sniffing to make sure there isn't anything suspicious going on. But, at the end of the day, the plugin has write access to the same files that will do the sniffing as well as all the objects on ghost to override and get around whatever we do. It's something that will require some extra thought to do non-superficially.

How would you envision we distribute the base classes?

I think we should distribute it as a separate module called ghost-plugin. If we absolutely must keep it in the Ghost repo it should be made much easier to require than the long nested path to the current GhostPlugin class.

Member

jgable commented Oct 23, 2013

I took a pass at simplifying the multiple inheritance you were doing in the PostPlugin file. You can see that here: gotdibbs/ghost-plugin@d5da5c5.

I like it, but I want to call it something like extendMultiple instead of define. I'll incorporate that in.

DatabasePlugin will probably need some significant foresight/security reviews when you consider their ability to add/drop schema. Maybe provide our own wrapper around knex.js?

I agree this is sort of wide open at the moment. Hannah brought up the idea of custom post data, which I like and can be incorporated into this class once it's fleshed out. But, I also like having the power to do some really advanced stuff with direct db access.

I have a similar security concern around giving plugins direct access to our server/express objects. Perhaps this is something we can wrap as well?

Right now we pass the server straight through, but we could make a proxy that exposes only the things we want to (get, post, use, etc). Then, those proxy handlers could also do some sniffing to make sure there isn't anything suspicious going on. But, at the end of the day, the plugin has write access to the same files that will do the sniffing as well as all the objects on ghost to override and get around whatever we do. It's something that will require some extra thought to do non-superficially.

How would you envision we distribute the base classes?

I think we should distribute it as a separate module called ghost-plugin. If we absolutely must keep it in the Ghost repo it should be made much easier to require than the long nested path to the current GhostPlugin class.

@gotdibbs

This comment has been minimized.

Show comment
Hide comment
@gotdibbs

gotdibbs Oct 23, 2013

Member

I took a pass at simplifying the multiple inheritance you were doing in the PostPlugin file. You can see that here: gotdibbs/ghost-plugin@d5da5c5.

I like it, but I want to call it something like extendMultiple instead of define. I'll incorporate that in.

You're right, I don't think I like define either. However, I think it would be cool to merge extend and extendMultiple into one convenience method so that you don't have to think if you want to add another base class to inherit from.

Another random related thought: in the stubs, it might be better to throw a "not implemented" error instead of just returning undefined but I could see it either way.

I agree this is sort of wide open at the moment. Hannah brought up the idea of custom post data, which I like and can be incorporated into this class once it's fleshed out. But, I also like having the power to do some really advanced stuff with direct db access.

With great power comes great responsibility. I was mainly worried about some random plugin being exploited so that it is able to come in and drop the entire posts table or something.

It's possible that the security stuff just needs to be handled by a plugin verification process or something on our end, but not sure.

Member

gotdibbs commented Oct 23, 2013

I took a pass at simplifying the multiple inheritance you were doing in the PostPlugin file. You can see that here: gotdibbs/ghost-plugin@d5da5c5.

I like it, but I want to call it something like extendMultiple instead of define. I'll incorporate that in.

You're right, I don't think I like define either. However, I think it would be cool to merge extend and extendMultiple into one convenience method so that you don't have to think if you want to add another base class to inherit from.

Another random related thought: in the stubs, it might be better to throw a "not implemented" error instead of just returning undefined but I could see it either way.

I agree this is sort of wide open at the moment. Hannah brought up the idea of custom post data, which I like and can be incorporated into this class once it's fleshed out. But, I also like having the power to do some really advanced stuff with direct db access.

With great power comes great responsibility. I was mainly worried about some random plugin being exploited so that it is able to come in and drop the entire posts table or something.

It's possible that the security stuff just needs to be handled by a plugin verification process or something on our end, but not sure.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Oct 27, 2013

Member

I haven't gotten into the nitty-gritty of the last couple of comments yet, but I wanted to reply more to the conceptual comment by @jgable.

This seems like it carries on perfectly from: https://github.com/TryGhost/Ghost/wiki/Imagining-Ghost-Themes-and-Plugins

I'm wondering if collaborating on a wiki page would be an easier way to flesh out all the basic kinds of interactions plugins are going to do?

There are two main kinds of plugins which I think are missing:

  1. Importer Plugins
  2. 3rd Party integration plugins

These may well conceptually be super or sub types of some of the ones @jgable mentioned but they are special because Importer Plugins are by far and away the first thing we need, and secondly because the concept of a 3rd Party integration plugin is something I want to provide support for in a very complete way, so that we don't end up with lots of disparate ones.

I have some paper notes on this somewhere which I'm looking for, and when I find I will add them to the wiki and comment here to let you know.

In short it boils down to the fact that in order to do this really well, we need to make sure we have support, in core, for working with different kinds of auth and API, then we need to provide tools to hook into different types of data - comments, stats, social, embed-able content etc, as well as be helpful in supporting both data pull and data push.

I think the integration with 3rd parties is almost as big as all the other kind of plugins combined.

There is another question around whether plugins should be extensible themselves. For example if we create a Facebook plugin which deals with FB comments, sharing, and likes, if someone wants to implement some other FB feature, should they extend the existing plugin, or recreate all the code from scratch. This depends a lot on what the toolset for connecting these things ends up like.

This is a bit of an unorganised brain dump, but I have so much stuff in my brain around this stuff, I figured I have to start somewhere!

Member

ErisDS commented Oct 27, 2013

I haven't gotten into the nitty-gritty of the last couple of comments yet, but I wanted to reply more to the conceptual comment by @jgable.

This seems like it carries on perfectly from: https://github.com/TryGhost/Ghost/wiki/Imagining-Ghost-Themes-and-Plugins

I'm wondering if collaborating on a wiki page would be an easier way to flesh out all the basic kinds of interactions plugins are going to do?

There are two main kinds of plugins which I think are missing:

  1. Importer Plugins
  2. 3rd Party integration plugins

These may well conceptually be super or sub types of some of the ones @jgable mentioned but they are special because Importer Plugins are by far and away the first thing we need, and secondly because the concept of a 3rd Party integration plugin is something I want to provide support for in a very complete way, so that we don't end up with lots of disparate ones.

I have some paper notes on this somewhere which I'm looking for, and when I find I will add them to the wiki and comment here to let you know.

In short it boils down to the fact that in order to do this really well, we need to make sure we have support, in core, for working with different kinds of auth and API, then we need to provide tools to hook into different types of data - comments, stats, social, embed-able content etc, as well as be helpful in supporting both data pull and data push.

I think the integration with 3rd parties is almost as big as all the other kind of plugins combined.

There is another question around whether plugins should be extensible themselves. For example if we create a Facebook plugin which deals with FB comments, sharing, and likes, if someone wants to implement some other FB feature, should they extend the existing plugin, or recreate all the code from scratch. This depends a lot on what the toolset for connecting these things ends up like.

This is a bit of an unorganised brain dump, but I have so much stuff in my brain around this stuff, I figured I have to start somewhere!

@grillorafael

This comment has been minimized.

Show comment
Hide comment
@grillorafael

grillorafael Oct 27, 2013

Contributor

[I've posted this in the wrong issue, sorry. I'm reposting it here]
About the Plugin structure and as a Developer, I think that a Plugin should behave like a Ruby Gem or a NPM Package but obeying a specific structure.

Wordpress plugins are really a mess and I think that the worst part of it is the lack of tests and their level of intrusiveness. I mean, it was really hard to trust that a plugin would not melt your whole blog.

In my personal opinion (which can be trash opinion), I would be more comfortable if the plugins environment be controlled. I mean, you guys should give to the developer lots of hooks se the developer can work the server side part of their plugin without writing code inside the core code.

The guys from Spree created something they called Deface, which is a greate unobtrusive way to override and add content to a specific view.

A Specific folder division would work well too (is not flexible but grants security).

Like Android Databases and Rails Migrates, It would be good if the developer can write some code to work with versions. Maybe a way to migrate data from a X version to a Y version. So the user trusts on updating a plugin.

Hope my ideas can help you guys somehow.

Contributor

grillorafael commented Oct 27, 2013

[I've posted this in the wrong issue, sorry. I'm reposting it here]
About the Plugin structure and as a Developer, I think that a Plugin should behave like a Ruby Gem or a NPM Package but obeying a specific structure.

Wordpress plugins are really a mess and I think that the worst part of it is the lack of tests and their level of intrusiveness. I mean, it was really hard to trust that a plugin would not melt your whole blog.

In my personal opinion (which can be trash opinion), I would be more comfortable if the plugins environment be controlled. I mean, you guys should give to the developer lots of hooks se the developer can work the server side part of their plugin without writing code inside the core code.

The guys from Spree created something they called Deface, which is a greate unobtrusive way to override and add content to a specific view.

A Specific folder division would work well too (is not flexible but grants security).

Like Android Databases and Rails Migrates, It would be good if the developer can write some code to work with versions. Maybe a way to migrate data from a X version to a Y version. So the user trusts on updating a plugin.

Hope my ideas can help you guys somehow.

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Oct 28, 2013

Member

@grillorafael

I think that a Plugin should behave like a Ruby Gem or a NPM Package but obeying a specific structure.

This is the plan.

I would be more comfortable if the plugins environment be controlled.

This is a discussion for John and Hannah and will probably take place outside of Github.

It would be good if the developer can write some code to work with versions

I like your forethought. I haven't really thought about upgrading plugins and that they would need to do migrations of their own. This is something that will need to be handled by the ghost-plugins repo and not necessarily core, unless core needs to call something on the plugin like upgrade() to kick off the migrations. I'd prefer to "kick the can down the road" on this one until we at least get the 0.1 version of plugins out.

Member

jgable commented Oct 28, 2013

@grillorafael

I think that a Plugin should behave like a Ruby Gem or a NPM Package but obeying a specific structure.

This is the plan.

I would be more comfortable if the plugins environment be controlled.

This is a discussion for John and Hannah and will probably take place outside of Github.

It would be good if the developer can write some code to work with versions

I like your forethought. I haven't really thought about upgrading plugins and that they would need to do migrations of their own. This is something that will need to be handled by the ghost-plugins repo and not necessarily core, unless core needs to call something on the plugin like upgrade() to kick off the migrations. I'd prefer to "kick the can down the road" on this one until we at least get the 0.1 version of plugins out.

@grillorafael

This comment has been minimized.

Show comment
Hide comment
@grillorafael

grillorafael Oct 28, 2013

Contributor

Sure @jgable . About the upgrades, that's something I think Wordpress fails. Users are always worried about updating plugins. And of course, I agree that its something that you guys should worry later. But thanks anyway for the attention. You guys are clearly working day and night on this!

Contributor

grillorafael commented Oct 28, 2013

Sure @jgable . About the upgrades, that's something I think Wordpress fails. Users are always worried about updating plugins. And of course, I agree that its something that you guys should worry later. But thanks anyway for the attention. You guys are clearly working day and night on this!

ErisDS added a commit that referenced this issue Oct 29, 2013

Plugin API Refactor: Filter and Theme Helpers
issue #769

- Refactor doFilter to allow returning a promise from a filter handler
  and to also return a promise itself
- Move the logic out of the registerThemeHelper calls and into their own methods so
  we could test them in isolation.
- Assign the server to the ghost instance so the initPlugins method can
  get access to it.

ErisDS added a commit that referenced this issue Oct 29, 2013

Read activePlugins from settings & improve error handling
issue #769

- activePlugins were being read from settings in two different ways, this has been simplified
- error handling has been improved so that plugins do not crash Ghost
- used full error messaging capabilities to make it easier to recover from errors
@zeroasterisk

This comment has been minimized.

Show comment
Hide comment
@zeroasterisk

zeroasterisk Nov 2, 2013

@grillorafael @jgable regarding plugin versioning and updating, the pattern I've seen most successful is to have all plugins be their own git repositories and a manager/configuration to either specify a tag/hash or keep up to date as latests on a branch (usually master).

A successful example of this is the community meteor package site https://atmosphere.meteor.com/wtf/app and the accompanying script http://oortcloud.github.io/meteorite/
(note: those are not plugins which would work with Ghost, obviously, but a pattern you may consider adopting)

zeroasterisk commented Nov 2, 2013

@grillorafael @jgable regarding plugin versioning and updating, the pattern I've seen most successful is to have all plugins be their own git repositories and a manager/configuration to either specify a tag/hash or keep up to date as latests on a branch (usually master).

A successful example of this is the community meteor package site https://atmosphere.meteor.com/wtf/app and the accompanying script http://oortcloud.github.io/meteorite/
(note: those are not plugins which would work with Ghost, obviously, but a pattern you may consider adopting)

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 2, 2013

Member

Plugin packaging, updating, versioning, all of that kind of thing is an entirely separate issue to what the API looks like, and depends heavily on what technology we choose for the official Ghost marketplace for plugins. There are a lot of valid ideas here, but I think they should probably be discussed somewhere else?

The reason being that this issue is focused on ensuring that the template or structure of a plugin makes sense, that the API for hooking into Ghost is sensible, and other similar ideas which are super duper important and require a lot of thought & input.

This issue contains example code and concepts that I'd really like to see more people feedback on.

Member

ErisDS commented Nov 2, 2013

Plugin packaging, updating, versioning, all of that kind of thing is an entirely separate issue to what the API looks like, and depends heavily on what technology we choose for the official Ghost marketplace for plugins. There are a lot of valid ideas here, but I think they should probably be discussed somewhere else?

The reason being that this issue is focused on ensuring that the template or structure of a plugin makes sense, that the API for hooking into Ghost is sensible, and other similar ideas which are super duper important and require a lot of thought & input.

This issue contains example code and concepts that I'd really like to see more people feedback on.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 3, 2013

Member

I have a few pretty big concerns with all this that I am trying to work through at the moment.

The main one is with:

Plugin = function (ghost) {

The object passed to plugins should be an object which gives controlled and specific access to Ghost via a well understood API - the Ghost object itself is (as we know from #360) a very bad idea and far too much of a jack-of-all-trades.

I'm also not sure if making all these different 'types' of plugins is the right approach, each of these types of plugin is more like a type of modification that a plugin might wish to make to Ghost, and it's likely that a plugin will do many of these things rather than one or two - it seems that each one is an aspect which the API to Ghost needs to provide methods for. I have a feeling that the plugin-type-approach may be a natural fall out from the fact we don't have the tools we need just yet, and that a different approach might have been natural if/when we do. I'm just speculating, but I don't think about much else ;)

It's also worth noting that we are intending to refer to 'plugins' as 'apps' from now on, including changing the name of the content/plugins directory by the time 0.4 comes around.

I guess my concern is that there is a lot more internal work that needs to happen in Ghost core for apps to really become a reality - and that that may be hamstringing us in ways we cannot imagine.

Member

ErisDS commented Nov 3, 2013

I have a few pretty big concerns with all this that I am trying to work through at the moment.

The main one is with:

Plugin = function (ghost) {

The object passed to plugins should be an object which gives controlled and specific access to Ghost via a well understood API - the Ghost object itself is (as we know from #360) a very bad idea and far too much of a jack-of-all-trades.

I'm also not sure if making all these different 'types' of plugins is the right approach, each of these types of plugin is more like a type of modification that a plugin might wish to make to Ghost, and it's likely that a plugin will do many of these things rather than one or two - it seems that each one is an aspect which the API to Ghost needs to provide methods for. I have a feeling that the plugin-type-approach may be a natural fall out from the fact we don't have the tools we need just yet, and that a different approach might have been natural if/when we do. I'm just speculating, but I don't think about much else ;)

It's also worth noting that we are intending to refer to 'plugins' as 'apps' from now on, including changing the name of the content/plugins directory by the time 0.4 comes around.

I guess my concern is that there is a lot more internal work that needs to happen in Ghost core for apps to really become a reality - and that that may be hamstringing us in ways we cannot imagine.

@aexmachina

This comment has been minimized.

Show comment
Hide comment
@aexmachina

aexmachina Nov 3, 2013

I think that it's premature to be developing a fully-featured plugin API at
this stage.

I think the approach that was used in the Drupal CMS provides a good
interim solution. This approach is that the framework exposes a number of
"hooks" that provide an extension point into the framework. We could
implement this using EventEmitters and provide a context object on the
event which provides information and some objects that can be used to
monkey with the processing.

This would allow the plugins, or apps if you prefer, to use the provided
objects in the event context and the public APIs of Ghost to influence
the way the system behaves. This would allow devs to innovate in userland
and then as we get closer to 1.0 we know what people are doing and we can
make improvements based on real-world usage, and to design fixes to common
problems.

This approach make make the plugins a bit more prone to problems when
upgrading, but it doesn't paint us in to a corner maintaining a published
"plugin API".

Thoughts?

aexmachina commented Nov 3, 2013

I think that it's premature to be developing a fully-featured plugin API at
this stage.

I think the approach that was used in the Drupal CMS provides a good
interim solution. This approach is that the framework exposes a number of
"hooks" that provide an extension point into the framework. We could
implement this using EventEmitters and provide a context object on the
event which provides information and some objects that can be used to
monkey with the processing.

This would allow the plugins, or apps if you prefer, to use the provided
objects in the event context and the public APIs of Ghost to influence
the way the system behaves. This would allow devs to innovate in userland
and then as we get closer to 1.0 we know what people are doing and we can
make improvements based on real-world usage, and to design fixes to common
problems.

This approach make make the plugins a bit more prone to problems when
upgrading, but it doesn't paint us in to a corner maintaining a published
"plugin API".

Thoughts?

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Nov 4, 2013

Member

Plugin = function (ghost) {

That ghost can be whatever we want to pass to it; I store it as app on the instance anyway. We already control what we pass that value as through the core/server/plugin initialization code. If we make it a facade that just exposes we want, that would be great. Everything is in place to start doing that, just needs to be implemented.

I'm also not sure if making all these different 'types' of plugins is the right approach, each of these types of plugin is more like a type of modification that a plugin might wish to make to Ghost, and it's likely that a plugin will do many of these things rather than one or two

They are meant to be the "building blocks" of plugins; they are broken down by separated concerns/logic. The ImportPlugin for instance only needs to register filters, and therefore doesn't need all the logic in the other plugin types. Whereas the PostPlugin does need all the other logic combined into one. I'll admit, most people won't need to use the lower level plugin base classes, the intent is to use them to build the higher level ones that people will inherit from; a la PostPlugin and ImportPlugin.

It's also worth noting that we are intending to refer to 'plugins' as 'apps' from now on, including changing the name of the content/plugins directory by the time 0.4 comes around.

I don't mind whatever you want to color that particular bike shed. I'll change my name to match whatever you'd like.

Member

jgable commented Nov 4, 2013

Plugin = function (ghost) {

That ghost can be whatever we want to pass to it; I store it as app on the instance anyway. We already control what we pass that value as through the core/server/plugin initialization code. If we make it a facade that just exposes we want, that would be great. Everything is in place to start doing that, just needs to be implemented.

I'm also not sure if making all these different 'types' of plugins is the right approach, each of these types of plugin is more like a type of modification that a plugin might wish to make to Ghost, and it's likely that a plugin will do many of these things rather than one or two

They are meant to be the "building blocks" of plugins; they are broken down by separated concerns/logic. The ImportPlugin for instance only needs to register filters, and therefore doesn't need all the logic in the other plugin types. Whereas the PostPlugin does need all the other logic combined into one. I'll admit, most people won't need to use the lower level plugin base classes, the intent is to use them to build the higher level ones that people will inherit from; a la PostPlugin and ImportPlugin.

It's also worth noting that we are intending to refer to 'plugins' as 'apps' from now on, including changing the name of the content/plugins directory by the time 0.4 comes around.

I don't mind whatever you want to color that particular bike shed. I'll change my name to match whatever you'd like.

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Nov 4, 2013

Member

@aexmachina

This approach is that the framework exposes a number of
"hooks" that provide an extension point into the framework.

We call those "filters" in Ghost and they function nearly identically to what your talking about. Search the code base for registerFilter and doFilter to see them in action.

Member

jgable commented Nov 4, 2013

@aexmachina

This approach is that the framework exposes a number of
"hooks" that provide an extension point into the framework.

We call those "filters" in Ghost and they function nearly identically to what your talking about. Search the code base for registerFilter and doFilter to see them in action.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 4, 2013

Member

@aexmachina As @jgable has rightly pointed out, we have had that possibility since before Ghost was public, in fact, since the very first prototype. We already have a lot of information about what users will and have done with that, and what they want to do - we are bombarded with app requests and ideas.

At this stage, it's not about building a fully-fledged app API - but ensuring we have a toolkit that developers can use to access the filters, and a few other aspects that are ready to have access provided, such as importing. The tools/api for building apps in 0.3.3 is not good enough. Mistakes now will be costly later, so this should be done whilst keeping in mind everything we do know about what the final API will look like.

@jgable - LOL... it's a brightly painted bikeshed, and it's all ours ;)

I find it amazing how little feedback we're getting on what the example Apps look like :/

Member

ErisDS commented Nov 4, 2013

@aexmachina As @jgable has rightly pointed out, we have had that possibility since before Ghost was public, in fact, since the very first prototype. We already have a lot of information about what users will and have done with that, and what they want to do - we are bombarded with app requests and ideas.

At this stage, it's not about building a fully-fledged app API - but ensuring we have a toolkit that developers can use to access the filters, and a few other aspects that are ready to have access provided, such as importing. The tools/api for building apps in 0.3.3 is not good enough. Mistakes now will be costly later, so this should be done whilst keeping in mind everything we do know about what the final API will look like.

@jgable - LOL... it's a brightly painted bikeshed, and it's all ours ;)

I find it amazing how little feedback we're getting on what the example Apps look like :/

jptacek added a commit to jptacek/Ghost that referenced this issue Nov 4, 2013

Plugin API Refactor: Filter and Theme Helpers
issue #769

- Refactor doFilter to allow returning a promise from a filter handler
  and to also return a promise itself
- Move the logic out of the registerThemeHelper calls and into their own methods so
  we could test them in isolation.
- Assign the server to the ghost instance so the initPlugins method can
  get access to it.

jptacek added a commit to jptacek/Ghost that referenced this issue Nov 4, 2013

Read activePlugins from settings & improve error handling
issue #769

- activePlugins were being read from settings in two different ways, this has been simplified
- error handling has been improved so that plugins do not crash Ghost
- used full error messaging capabilities to make it easier to recover from errors
@aexmachina

This comment has been minimized.

Show comment
Hide comment
@aexmachina

aexmachina Nov 4, 2013

@ErisDS thanks for the response. Okay that sounds good. just putting my 10 cents in. Can you point me in a direction where I can see the current state of the apps API?

Example apps? I wasn't aware there were any! Where can I find these?

aexmachina commented Nov 4, 2013

@ErisDS thanks for the response. Okay that sounds good. just putting my 10 cents in. Can you point me in a direction where I can see the current state of the apps API?

Example apps? I wasn't aware there were any! Where can I find these?

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 5, 2013

Member

@aexmachina I don't mean to be funny, but it's all right here in the comments of this issue!

Member

ErisDS commented Nov 5, 2013

@aexmachina I don't mean to be funny, but it's all right here in the comments of this issue!

@aexmachina

This comment has been minimized.

Show comment
Hide comment
@aexmachina

aexmachina Nov 5, 2013

Can anyone point me towards the apps API and example app?

aexmachina commented Nov 5, 2013

Can anyone point me towards the apps API and example app?

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Nov 5, 2013

Member

@aexmachina There is no official api, but here are some extremely early prototypes (right in this thread even) based purely on speculation.

It's likely most of those Base Plugin Types will be reduced/eliminated based on a new direction that will rely more heavily on filters.

Member

jgable commented Nov 5, 2013

@aexmachina There is no official api, but here are some extremely early prototypes (right in this thread even) based purely on speculation.

It's likely most of those Base Plugin Types will be reduced/eliminated based on a new direction that will rely more heavily on filters.

@sebgie

This comment has been minimized.

Show comment
Hide comment
@sebgie

sebgie Nov 10, 2013

Contributor

With issue #755 I have the plan to introduce ghost.api() to have a single access point for database operations. Follwing this discussion I think that we could use ghost.api() to abstract the access to Ghost for apps.

The tricky part is that we need to support three different access methods: apps, internal, REST.
If ghost.api() is implemented in a way that distinguishes between the three access methods, the functionality could be reused.

Data API

API method App REST Internal
ghost.api.posts x x x
ghost.api.users x x x
ghost.api.tags x x x
ghost.api.notifications x x x
ghost.api.settings x x x
ghost.api.db x x x

Apps API

API method App REST Internal
ghost.api.registerFilter x x
ghost.api.unregisterFilter x x
ghost.api.registerThemeHelper x x
ghost.api.registerAsyncThemeHelper x x
...

It is probably not needed in 0.4, but with this API structure it would be possible to introduce a security layer, where an app has to ask for authorization to access certain APIs. The user can grant access and he would have confidence that an App is not messing with data it is not allowed to access (e.g.: Kudos app doesn't need access to userdata).

I also think it would be convenient to have an API to simplify database access. Schema generation could use the same functionality implemented with issue #1398. What would need a bit of research in my opinion is dynamic registration of bookshelf models, but I am convinced that this is doable. In a first step the base methods from core/server/models/base.js could be used to provide CRUD functionality to plugins.

DB management api example

var kudos = {
    kudos: {
    id: {type:'incremental', primary:true},
    postid: {type: 'integer', nullable:false},
    kudos: {type: 'integer', defaultTo:0}
    }
};
ghost.api.registerDataModel(kudos);
ghost.api.kudos.get({postid:1});

Contributor

sebgie commented Nov 10, 2013

With issue #755 I have the plan to introduce ghost.api() to have a single access point for database operations. Follwing this discussion I think that we could use ghost.api() to abstract the access to Ghost for apps.

The tricky part is that we need to support three different access methods: apps, internal, REST.
If ghost.api() is implemented in a way that distinguishes between the three access methods, the functionality could be reused.

Data API

API method App REST Internal
ghost.api.posts x x x
ghost.api.users x x x
ghost.api.tags x x x
ghost.api.notifications x x x
ghost.api.settings x x x
ghost.api.db x x x

Apps API

API method App REST Internal
ghost.api.registerFilter x x
ghost.api.unregisterFilter x x
ghost.api.registerThemeHelper x x
ghost.api.registerAsyncThemeHelper x x
...

It is probably not needed in 0.4, but with this API structure it would be possible to introduce a security layer, where an app has to ask for authorization to access certain APIs. The user can grant access and he would have confidence that an App is not messing with data it is not allowed to access (e.g.: Kudos app doesn't need access to userdata).

I also think it would be convenient to have an API to simplify database access. Schema generation could use the same functionality implemented with issue #1398. What would need a bit of research in my opinion is dynamic registration of bookshelf models, but I am convinced that this is doable. In a first step the base methods from core/server/models/base.js could be used to provide CRUD functionality to plugins.

DB management api example

var kudos = {
    kudos: {
    id: {type:'incremental', primary:true},
    postid: {type: 'integer', nullable:false},
    kudos: {type: 'integer', defaultTo:0}
    }
};
ghost.api.registerDataModel(kudos);
ghost.api.kudos.get({postid:1});

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Nov 11, 2013

Member

@sebgie That all looks good to me. Hannah mentioned two things that I think will affect both the ghost.api and DB management stuff.

First, she mentioned passing a "facade" object to the App initialization/activate methods that we could lock down to prevent "inappropriate" access. Which I'm not sure if that really affects the ghost.api stuff or not since it's already kind of wrapped.

Second, she mentioned having a "Custom Data" table for Apps but I'm not sure what kind of work needs to be done for that and whether we'd still want something like you describe for more advanced use cases.

Honestly, I'm not sure where this issue stands right now. I need an adult to tell me what to do.

Member

jgable commented Nov 11, 2013

@sebgie That all looks good to me. Hannah mentioned two things that I think will affect both the ghost.api and DB management stuff.

First, she mentioned passing a "facade" object to the App initialization/activate methods that we could lock down to prevent "inappropriate" access. Which I'm not sure if that really affects the ghost.api stuff or not since it's already kind of wrapped.

Second, she mentioned having a "Custom Data" table for Apps but I'm not sure what kind of work needs to be done for that and whether we'd still want something like you describe for more advanced use cases.

Honestly, I'm not sure where this issue stands right now. I need an adult to tell me what to do.

@hswolff

This comment has been minimized.

Show comment
Hide comment
@hswolff

hswolff Nov 11, 2013

Member

I'm in pretty much full agreement with what you proposed @sebgie. The greater the separation of concerns will produce a stronger code base. Having one point of truth for DB access that is both used by REST and the App would go a long way to adding to Ghost's overall robustness.

Member

hswolff commented Nov 11, 2013

I'm in pretty much full agreement with what you proposed @sebgie. The greater the separation of concerns will produce a stronger code base. Having one point of truth for DB access that is both used by REST and the App would go a long way to adding to Ghost's overall robustness.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 13, 2013

Member

I think there is a little bit of confusion here, between what the ghost and api objects do, and what they are called, and what they relate to in the code base, mostly cos it is confusing and a bit of a mess.

There should be an object which is passed into plugins/apps. At the moment this is the ghost object from the ghost.js file, which is entirely completely and totally wrong - but it's all we have right now. What this should be is 'the thing which gives the application access to ghost', which although it is an 'Application programming interface' by definition, it is more than this because it is a collection of distinct APIs.

Lets call it the SuperAPI for now.

That SuperAPI, is a collection of distinct apis, which you might access to do different things.

For example

SuperAPI.routes.add('get', 'my-subdir/my-url', doSomethingFunction);

Or maybe

SuperAPI.services.add('twitter', twitter-username, some-credential);

Or many other things.

One of the things the SuperAPI has access to is data. At the moment, data is accessible through an internal object in Ghost called 'api'. If we kept this name we'd have

SuperAPI.api.posts(...)

But perhaps we need to rename it to 'data'

SuperAPI.data.posts(...)

But it definitely doesn't make sense in my opinion for it to be

SuperAPI.posts(...)

And equally, I don't think there's any need for 2 things which are 'the api'

SuperAPI.api.routes.add(...) and SuperAPI.api.post.read(...)

Personally, I'm happy to keep the data api being called api and call the super object ghost, because ghost in this context would be short for Ghost Developer Tools which is the name we have currently settled on for the whole kit and caboodle of apis we'll be providing for Apps.
However, it may make sense to rename it data inside the GDK if not inside Ghost as well. We're back to the whole 'naming things is hard' problem IMO, and I find it VERY hard so I tend to make @JohnONolan come up with all the names for stuff.

Member

ErisDS commented Nov 13, 2013

I think there is a little bit of confusion here, between what the ghost and api objects do, and what they are called, and what they relate to in the code base, mostly cos it is confusing and a bit of a mess.

There should be an object which is passed into plugins/apps. At the moment this is the ghost object from the ghost.js file, which is entirely completely and totally wrong - but it's all we have right now. What this should be is 'the thing which gives the application access to ghost', which although it is an 'Application programming interface' by definition, it is more than this because it is a collection of distinct APIs.

Lets call it the SuperAPI for now.

That SuperAPI, is a collection of distinct apis, which you might access to do different things.

For example

SuperAPI.routes.add('get', 'my-subdir/my-url', doSomethingFunction);

Or maybe

SuperAPI.services.add('twitter', twitter-username, some-credential);

Or many other things.

One of the things the SuperAPI has access to is data. At the moment, data is accessible through an internal object in Ghost called 'api'. If we kept this name we'd have

SuperAPI.api.posts(...)

But perhaps we need to rename it to 'data'

SuperAPI.data.posts(...)

But it definitely doesn't make sense in my opinion for it to be

SuperAPI.posts(...)

And equally, I don't think there's any need for 2 things which are 'the api'

SuperAPI.api.routes.add(...) and SuperAPI.api.post.read(...)

Personally, I'm happy to keep the data api being called api and call the super object ghost, because ghost in this context would be short for Ghost Developer Tools which is the name we have currently settled on for the whole kit and caboodle of apis we'll be providing for Apps.
However, it may make sense to rename it data inside the GDK if not inside Ghost as well. We're back to the whole 'naming things is hard' problem IMO, and I find it VERY hard so I tend to make @JohnONolan come up with all the names for stuff.

@jgable

This comment has been minimized.

Show comment
Hide comment
@jgable

jgable Nov 13, 2013

Member

I propose we close this issue and make a new one with the tasks you'd like completed. Seems like we could start with:

  • Define the proxy object name and functionality that will be passed to Apps (SuperAPI or ghost)
  • Refactor code to pass defined proxy object instead of ghost instance
  • Refactor code to not pass the server directly and instead use a filter to register routes if necessary
  • Define whether we want custom post data tables or some other data table creation functionality like what @sebgie proposed above with ghost.api.registerDataModel(kudos);. Or if this is even necessary for v1 of Apps.

Then of course, the UI work for activating and installing.

Member

jgable commented Nov 13, 2013

I propose we close this issue and make a new one with the tasks you'd like completed. Seems like we could start with:

  • Define the proxy object name and functionality that will be passed to Apps (SuperAPI or ghost)
  • Refactor code to pass defined proxy object instead of ghost instance
  • Refactor code to not pass the server directly and instead use a filter to register routes if necessary
  • Define whether we want custom post data tables or some other data table creation functionality like what @sebgie proposed above with ghost.api.registerDataModel(kudos);. Or if this is even necessary for v1 of Apps.

Then of course, the UI work for activating and installing.

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 13, 2013

Member

I am in the process of doing exactly this :)

Member

ErisDS commented Nov 13, 2013

I am in the process of doing exactly this :)

@ErisDS ErisDS closed this Nov 13, 2013

@aexmachina

This comment has been minimized.

Show comment
Hide comment
@aexmachina

aexmachina Nov 13, 2013

So which issue has this discussion moved to?

aexmachina commented Nov 13, 2013

So which issue has this discussion moved to?

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Nov 14, 2013

Member

Different parts of the discussion will move to different issues across different repositories.

It'll all be kept track of here: #1474

Member

ErisDS commented Nov 14, 2013

Different parts of the discussion will move to different issues across different repositories.

It'll all be kept track of here: #1474

morficus pushed a commit to morficus/Ghost that referenced this issue Sep 4, 2014

Plugin API Refactor: Filter and Theme Helpers
issue #769

- Refactor doFilter to allow returning a promise from a filter handler
  and to also return a promise itself
- Move the logic out of the registerThemeHelper calls and into their own methods so
  we could test them in isolation.
- Assign the server to the ghost instance so the initPlugins method can
  get access to it.

morficus pushed a commit to morficus/Ghost that referenced this issue Sep 4, 2014

Read activePlugins from settings & improve error handling
issue #769

- activePlugins were being read from settings in two different ways, this has been simplified
- error handling has been improved so that plugins do not crash Ghost
- used full error messaging capabilities to make it easier to recover from errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment