Opening some discussion on the app structure

[tgriesser]

So these are just some ramblings I have about development style/structure for the frontend and backend, along with sort of a few few minutes of playing around with a few files... disregard most of the code, except maybe this file, it's not intended to be merged (nothing works most likely, I don't actually have ghost running while doing this stuff). This is more of just a sandbox to show some ideas/get some discussion going around with code style/app structure.

So coding for the browser and node are sort of two different environments, and while they're both javascript, they sort of have two different purposes...

When dealing with the actual server side logic (not libs/modules that are shared between the two), I find it a bit clearer to embrace the ability to declare things as/where necessary, using the exports object (where it makes sense - maybe use module.exports if you're exporting a constructor)... but if we're going to keep the app as a singleton for now, we might as well just use the containing scope of the main file as the module - exports is cached on subsequent requires - as opposed to having the if (!instance) check (forced singletons usually feel like a bit of an anti-pattern anyway)... Then we can easily add the ability to create multiple instance easily later if need be...

These are sort of the general style guide for node apps:

https://npmjs.org/doc/coding-style.html
http://nodeguide.com/style.html

I didn't know if it was worth considering following some of these for the server and then using whatever else we'd like for the client... part of my reasoning being that since the node aspect of the project is heavily promoted, it might make sense to follow patterns familiar to node developers to encourage contribution.

I also find that the node guidelines tends to make the app structure simpler... the module patterns makes a lot of sense when you explicitly want to prevent people from fiddling with the innards of the application - but if we're making this open, we'd probably want to make it easier to do that, correct? Or maybe not, but it'd be good to figure out what we should/shouldn't make easy to fiddle with.

In my opinion, "use strict" is overrated - it's more of something you should be doing when writing good javascript, then needing to require a top level wrapper, and as far as I've been able to find researching it, adding the actual "use strict" declaration doesn't offer any performance gains. I'm also sure it's pretty simple too hook into whatever test suite we're going to be using to actually validate against strict mode when running unit tests on the backend.

I also wasn't sure if it made sense to try to abstract everything from the beginning (i.e. with the dataprovider module)... that seems like it'd be what the ORM layer would be for for, and it might make sense to keep things really simple to start out with (simple routes handlers, simple models, etc) and then build up the abstractions once we see what the patterns are (controllers, dataprovider instances).

And finally - having done it a few times both ways, I think that it's easier to keep the filesystem grouped by type:

/core
  /controllers
    - admin.js
    - frontend.js
  /model
    - data.js
  /views
    /admin
       - admin_file.hbs
    /frontend
       - frontend_file.hbs 

It just seems easier to group conceptually, since then you don't need to think about which pieces of the data model only apply to the backend, or frontend, or shared... and the assets, etc. are going to be likely concatenated and moved around anyway, so those could live outside of the folders.

Just some ideas (and a few minutes of playing around with the code) I thought I'd put out there for some discussion... I'm probably going to keep fiddling with different pieces of the app to see what I come up with.

Sorry this was sort of a lot, I'm just looking start some discussion and get any feedback... and I won't take it personally if everyone disagrees with all of this :)

[jgable]

I'd agree with most of what you are saying here. However, I don't think the structure and style differences are big enough problems to warrant addressing right now before getting something out that the community can see. As long as we had a roadmap to address some concerns people would probably understand.

I will admit that as a node developer I would be turned off by the closure wrapped node code and "use strict" statements, but I can suppress that as a style thing.

I think the splitting up of the theme from the admin view is a good idea. It will let other people develop themes in isolation without worrying about the Ghost code base.

Just my 2 cents.

[ErisDS]

The code structure being separated into "core" and "content" is very important. We need an area that the average user can safely see as theirs for all their plugins, themes, images and any other custom content. It is that divide which drives the others. If we were working in one of a hundred other languages/frameworks, the admin and frontend would be two separate apps, but this is not usual for express. My thinking is that they are separate and share very little... this may turn out to not be true, we'll see. The internal structure of the two apps + the shared stuff leaves a lot to be desired at the moment though.

I am/was concerned about having different style guidelines between the front and backend as I think it adds complexity, and additionally, my problem with JSHint is that so many rules are off by default, so to get everyone coding from the same hymn sheet there are a tonne of settings which have to be wrangled. However, the argument to write code which node developers will be familiar and comfortable with is a strong one. To that end I am reviewing the validation rules and tools and will update everyone soon - however, I am far more focused on delivering the core features, so for now please stick to the current standards.

The way node code tends to use "exports" throughout a file sits funny with me. I'm used to very strictly providing the contract/public API of a module as the last thing in the file, very clearly separated from the logic and therefore to me having the exports functions littered throughout a file seems inherently wrong. I realise this is the norm for node code, but I can't help but wonder if there's not a better way to keep the logic and the contract separated?

However, I completely agree on the singleton being an anti-pattern & want to re-factor ghost.js at some point very soon. Also, the api / data provider / ORM abstraction is a little over the top but just for the moment it is serving a purpose, so I want to run with it for a bit longer.

[ricardobeat]

I agree with most of what @tgriesser said, apart from a few caveats.

The separation of core/content is fine, it aligns with the Wordpress model of downloading the source and keeping edits to a contained area. Usage via npm install/require can be supported regardless, and should be encouraged.

On JSLint etc, it's a touchy subject, but I'm of the opinion that jslint is too opinionated :) Basic rules like 4 spaces indent, no comma-first, do not spare semi-colons, do not return from constructors, are enough for a consistent codebase, and easy to set up with JSHint. Linting rules should be delegated to build tools, not individual files.

The (function(){ "use strict" }() wrappers should also be delegated to the build tool, browserify, requirejs or similar, allowing the backend code to be free of it. I think it's something that should be dealt with now, before the project starts getting more active and receive outside contributions, as merges with a large surface area will only get more complicated. I don't feel strongly about "use strict" since the code is already lint'ed.

I'm with @ErisDS on exports usage at the end of the file. It's just a special case of the "revealing module pattern", makes it very clear what methods/objects are being exposed. It looks redundant but that's just because javascript doesn't support destructuring :)

The most glaring anti-pattern right now for me is the ghost.app() and similar getters. We can have real getters using Object.defineProperty, but I don't see why not simply expose the objects directly as in ghost.app. Ideally, ghost should be framework-agnostic and all express-related code contained to app.js. This could be achieved simply by exporting routes and rendering views with handlebars directly. But I digress, this is something that can be done at a later stage.

Tea, anyone?

[tgriesser]

I agree with separating core/content so it's very clear/logical where the user should edit things... I was guess I was more referring to splitting the core into more of a client / api-server rather than grouping both the admin and public code into individual directories - I think there should be a decent amount of code shared between the two, especially if the frontend is consuming the backend's api, language stuff, ACL, etc... it sort of tends to blur together in terms of the admin / frontend server code.

Re: the node stuff, the reason this works is because you can easily split things up into individual files... you don't have to have large bodies of code that only define a public accessible interface at the bottom... Also, you could sort of think of it like the module pattern, except the "exports" object defined as the object for the public interface - which is what happens... node wraps files like this during execution:

(function (exports, require, module, __filename, __dirname) { 
});

Maybe it's a preference thing, I find it easier to see what's publicly exposed while reading through rather than needing to jump between the code and the bottom of the file... unless you're just exposing a constructor, or a single function that sort of acts as constructor... or a hash of values like in the config in the other pull request. Then it totally makes sense to use module.exports.

I think in terms of the JSHint stuff, we could define a .jshintrc file in the base of the core/client and the base of the core/server (or however it gets structured) with the rules we're going to abide by and then that way we can make sure people stick to them...

From http://www.jshint.com/docs/#usage

use a special file .jshintrc. JSHint will look for this file in the current working directory and, if not found, will move one level up the directory tree all the way up to the filesystem root.

Just some ideas...

[tgriesser]

Just to make sure that was clear about the dir structure... this is what I was imagining it could look like to provide the separation and flexibility we're looking for to give overrides but also give a good layer of separation between the client and server, from the root:

|- themes
|   |- casper (theme structure mirrors core to some degree)
|       |- client
|       |   |- config.js (overrides/additions for core/server/config.js)
|       |   ... other stuff
|       |  
|       |- server
|       |   |- config.js (overrides/additions for core/server/config.js)
|       |   ...other stuff
|       |  
|       |- shared
|
|- core
|   |- client (code for browser execution)
|   |   |- frontend
|   |   |- images
|   |   |- scripts
|   |   |- config.js (default config for client)
|   |   -.jshintrc
|   |
|   |- server (code for node execution)
|   |   |- lib
|   |   |  |- express_setup.js (handles express setup, hooks)
|   |   |  |- database_setup.js (handles database/orm setup, hooks)
|   |   |  ...etc
|   |   |
|   |   |- controllers
|   |   |  |- frontend.js (frontend routes & controllers)
|   |   |  |- admin.js (admin routes & controllers)
|   |   |  |- superadmin.js (superadmin? routes & controllers)
|   |   |  ...etc
|   |   |
|   |   |- data
|   |   |  |- posts.js
|   |   |  |- accounts.js
|   |   |  |- plugins.js
|   |   |  ...etc
|   |   |
|   |   |- tmpl (holds templates for different app components, can be overwritten/hooked to modify... possibly moved
|   |   |        to shared if we're doing client side rendering too which would be great)
|   |   |  |- frontend 
|   |   |  |- backend
|   |   |
|   |   |- config.js (default config for server)
|   |   .jshintrc
|   |   index.js - load core things as necessary, provide main hooks/config loading
|   |  
|   |- shared (code that could be executed anywhere)
|       |- i18n
|       |- helpers
|       |- validation
|       ...etc
|
|
|- plugins
|   |- setup tbd
|

[ricardobeat]

@tgriesser I see. That looks good. I admit that the current structure confused me a bit at first (admin vs frontend).

If we turned the admin interface into a purely client-side app, the separation becomes more evident:

server:
  API
  public blog / pages
client:
  admin interface (backbone, rendering handlebars on the client, backed by the API)
  scripts shared with the public blog

[ErisDS]

With regards to JSLint/Hint and the function wrapper: I will review this asap, (I've created #58 as part of this milestone). However, I have several other higher priority things to do first - including getting the data model out for review, and writing up the remaining issues for milestone 0.2 - and I see absolutely no reason why the current standards should hold up feature development. Therefore, until I have done it please ensure all pull requests pass the current grunt validate.

With regards to the folder structure, they say there are only 2 hard problems in programming: cache busting, naming things, and off by one errors. This is why, it is my approach that naming things is as much a part of the design of the application as it is part of the development of the application.
The folder structure needs to make sense, first and foremost, to the less technical users and contributors first, not only because they are an important part of the Ghost community, but because as you both have proven - even if the structure is confusing to developers at first, they at least have a chance of figuring it out.

I believe having folders inside of content named "client" and "server" is going to scare the CRAP out of many theme & plugin designers / developers. It is definitely true that the current one is far from perfect, and there are certainly parts of these proposals which I think we could adopt. Also, moving "content" folders to the root breaks the notion of users having their own space. Equally, whilst the idea of architecting themes such that they extend an internal set of templates, this would further the need for the codebase to be designed to be understandable by users first as they would need to go poking around the internals of Ghost to understand how to write themes. I'm not at all sure that this is a good idea.

[ricardobeat]

@ErisDS do we have anything written down on themes/plugins?

We should tackle one thing at a time: shared folder, I can't help thinking of it as shared between server/client, when it's actually shared between admin/frontend. it's contents could live in /core, no need for a subfolder. Database files should be put in $ROOT/data instead of along the source. Should I send a PR for this?

[ErisDS]

@ricardobeat what sort of thing on themes/plugins are you after? There is an open issue (#46) assigned to @JohnONolan for him to take a write up we did for WooThemes/Envato on writing themes and add it to the wiki, which should be available soon. Do you have anything else specific that you are after?

With regards to shared, I'm happy for you to send a PR for this change. However, can we leave the database files where they are for now as I believe this is a separate issue. They are where they are to keep them as far from the user as possible. I'm thinking that the filename should possibly be donotdelete.db!

[ricardobeat]

The recommended way of using Ghost should be to create a new project:

// index.js
var Ghost = require('ghost')
  , Space = require('ghost-theme-spaceodissey')

new Ghost({
    theme: Space
}).listen(process.env.PORT || 2001)

// package.json
{
  name: "MyBlog",
  private: true,
  dependencies: {
    "ghost": "~v1.0",
    "ghost-theme-spaceodissey": "~v0.1"
  }
}

You just deploy this to your server and do npm install, tada! This should keep everyone away from trouble, and make sure Ghost exposes a comprehensible API.

[ricardobeat]

That can be distributed as a template, so you can fork it, clone it, and deploy!

[tgriesser]

What are anyones thoughts on having something similar to express and using commander to create a ghost bin file that would scaffold the project and install the ghost app...

So you'd do $ ghost tims-blog

And it'd create this:

/tims-blog
  /themes
    /casper
      ...
  /node_modules
  index.js
  config.js

and an npm install would install ghost & all related dependencies in node_modules, where index.js would look something like what @ricardobeat described above.

The naming for the theme directories doesn't need to be client / server, I agree that could be confusing. I may have misunderstood based on the current directory structure what all the theme / plugin designers/developers are going to need to have access to and hook into - it was more the notion of admin/ frontend/ shared folders in the core that was confusing to me.

I think express might be a good model for how something like this could look to be friendly to designers and devs... You install it, forget that it's there, call the necessary public hooks that you need and everything just works^TM.

var Ghost = require('ghost');

var ghost = new Ghost();

ghost.setConfig('./config');
ghost.setTheme('casper');

// Packages could hook into both the client & server.
ghost.addPackage('ghost-facebook');

ghost.addPlugin('./local/path/to/plugin')
ghost.beforeRender(function(html) {
   return html.replace('!', '!!');
});

ghost.addRoute(/your-route/, function(req, res) {
   res.send(...);
});

ghost.admin.setTheme('new-ghost-admin');
ghost.admin.addPlugin('./local/path/to/admin/plugin');
ghost.listen(process.env.PORT || 2001);

[JohnONolan]

The root directory structure is incredibly important to users being able to find their way around - particularly those migrating from WordPress and similar/older systems. The average user/developer (as in, those in the majority), in my experience with WordPress is someone who doesn't necessarily understand all the intricacies of an application structure. Which is to say that the terms "models / views / controllers" are worse than meaningless - they're actually scary.

I'm all for refactoring structure to make it make sense, particularly making it make sense within the Node / JS ecosystem. The root level of that structure - however - is one that I think needs to set some trends rather than follow them.

Dividing the root into core/content is an important distinction. Refactoring the level below 'core' however, is certainly something that I think we could look at if it makes sense.

[ErisDS]

I think express might be a good model for how something like this could look to be friendly to designers and devs... You install it, forget that it's there, call the necessary public hooks that you need and everything just worksTM.
Devs would get it, designers might, my mum wouldn't!

[ricardobeat]

I'll replace 'your mum' with "Dorothy" so I don't get into trouble :)

Devs would get it, designers might, my mum wouldn't!

But will Dorothy learn about downloading a .zip, unpacking and uploading to an FTP server with specialized software? node hosting services usually don't even offer FTP, since app lifecycle is different (long-lived process vs per-request).

I think this is where tryghost.org managed hosting comes in. Users who can download a WP release, setup a server & MySQL, edit wp-config.php and configure permalinks and plugins won't have any trouble running ghost init && npm start (or double-clicking a .sh/.bat file). Others can, and should be encouraged to, use the hosted version.

[ricardobeat]

Actually, it would be extremely easy to create a cross-platform GUI installer that you just download and point to a folder.

On further thought, the app.js (manually created or using the ghost tool) would be ghost's equivalent of wp-config.php.

[javorszky]

Taking the side of a theme or plugin developer, I would want to write my plugin (interchangable for theme from now on) to be self contained. I do not want to poke around app.js or ghost.js to have Ghost register my plugin. There should be hooks and filters in place that I can just point one of my plugin's export bits, and Ghost would pick it up, much like "is there a *.php with the necessary info in a comment block in a folder within wp-content/plugins? Yes? Awesome. 'Tis a plugin then!"

Say, API docs would say: "To have your plugin picked up by Ghost, do a module.exports.registerPlugin = initdata;" or something similar.

This also means that the core of Ghost will have to be littered with hooks and filters that would achieve these things.

[ricardobeat]

Ghost can automatically glob a /plugins folder and require() everything in there, without the need for explicitly registering them.

[ErisDS]

This is exactly the plan - ghost will read the themes & plugins folders to offer the ability to turn a theme/plugin on or off.

Although there will likely be underlying CLI tools for doing this sort of thing, everything will be available through a UI inside Ghost. There should be no download/upload/install process required.

[ErisDS]

I'd like to summarize and close this, moving any unresolved issues to telescope.

I think the main unresolved that hasn't continue elsewhere is the app structure, and I am of the opinion that we'll be able to see more clearly what's needed after we fall out from converting the admin to use backbone - so does anyone object to revisiting at that point?

[javorszky]

Nah, let's add some backbone first, and see where does it hurt after.

[ErisDS]

Closing this in lieu of further conversation once we have a clearer idea how the admin client side will look