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

Modularise the client side code #584

Closed
nicoburns opened this issue Aug 31, 2013 · 17 comments
Closed

Modularise the client side code #584

nicoburns opened this issue Aug 31, 2013 · 17 comments
Labels
affects:admin Anything relating to Ghost Admin
Milestone

Comments

@nicoburns
Copy link
Contributor

Node has a lovely module system, unfortunately browsers don't (yet). However, in my experience large single-page apps really need one. There are a number of different ways of doing this

Option 1: AMD modules

Advantages:

  • Modular code!
  • No compile step needed during development
  • The requireJS optimiser can combine module into one file for production (really needed for performance (especially on mobile)
  • AMD loaders support commonJS module syntax (with a two line wrapper). Useful for reusing code between client and server.

Disadvantages:

  • Learning curve: we could possibly discourage contributions with the added complexity.
  • Boilerplate code (although it's really not that much)

I'll keep adding more options as they are suggested. What do people think?

@jgable
Copy link
Contributor

jgable commented Aug 31, 2013

One problem for this is how to load assets for plugins and themes. I
believe one of the main goals for the code base is to make it easy for
designers to approach and extend. Unfortunately requires is one of those
things that while easy once you get it, will be a barrier to entry for most
beginners to understand.

@ErisDS
Copy link
Member

ErisDS commented Aug 31, 2013

My main issue with requirejs is not the complexity. I think that ship might have already sailed 😉 For people not accustomed to large JS applications, coming from a background of writing jQuery for WP themes, I think that we already have a somewhat overwhelming system.

My concern is the limitations around combining into one single file. It does depend somewhat on how much JS we have in total, but it always seems odd to me to combine everything.. for example only one page in the whole app needs all of the codemirror and showdown stuff. I prefer to chunk the code up into logical groups, and in the past I've had to work with custom code for doing this in a sane way with requireJS.

I don't think amd/requireJS are completely off the table, but I'd like to see some discussion around the alternatives?

@nicoburns
Copy link
Contributor Author

I think a lot of these things can be done with AMD (it allows for multiple files). We could take an approach similar to the BBC (see: http://responsivenews.co.uk/post/21021136520/how-we-build-our-javascript). From a purely mobile performance perspective I think it would be best to load core modules + initial page modules as one file on page load, and then load one file per page change. This could be achieved by running the requirejs optimiser dynamically. With caching this could be OK performance wise.

However, now that I think about it, there might well be better alternatives. I've rewritten the issue to be more generally about modularisation. My impression is that AMD is currently the best of a bad bunch. But there could well be other solutions out there that I haven't seen, or we could write our own.

@ErisDS
Copy link
Member

ErisDS commented Sep 2, 2013

The problem which we absolutely have to solve is compiling multiple files into a sensible set of files (issue #359). Loading the correct ones is still reasonably simple as we have a small app right now. I'm in favour of introducing the simplest possible solution to start with and working up to big tools like require.js when they become necessary.

We already have a compile step & tools for watching / autoreloading, so I see no reason why adding a new task to grunt would be a problem.

At the same time, the code is not too far away from being AMD-style, the only difference would be specifying the modules a file depends on, which I took out of the iifw because it hadn't been used consistently.

@nicoburns
Copy link
Contributor Author

Hmm... the way I see it, there are 3 significant JS module systems:

  • AMD
  • CommonJS
  • ES6 harmony modules

ES6 modules are out (for now), because they have zero support (except for the ES6 module transpiler, but it's own README advises against using it at the moment). CommonJS modules (unwrapped) only work on the server, which leaves AMD modules (which are basically CommonJS modules wrapped for client side code*)

I do agree that RequireJS seems overkill at the moment, but I see no advantages to writing our own module system when there is an established module system with sophisticated build tools, documentation, and wide 3rd party library support. I feel like we will end up reimplementing things which already exist otherwise (which would be a shame for a project like Ghost which generally seems to be good at building upon standard 3rd party libraries (backbone, express, etc).

* Assuming that we used wrapped CommonJS style AMD (Both RequireJS and Curl support this):

define(function(require, exports, module) {
    var _ = require("lodash");
});

@nicoburns
Copy link
Contributor Author

Whichever module system we end up with, I think modules should specify their dependencies. Otherwise we will have to build this information into the tool which compiles them together, which semi-defeats the point of modules.

P.S. Some relevant articles:
http://requirejs.org/docs/whyamd.html
http://tomdale.net/2012/01/amd-is-not-the-answer/ (also see comments)
http://know.cujojs.com/blog/the-future-looks-bright-for-modules-but-what-about-curl/

@ErisDS
Copy link
Member

ErisDS commented Sep 4, 2013

I'm not really sure why we need to use a specific module system. What do we gain at this point? Each piece of code is self contained and modular in nature, it just doesn't use any inter-module dependency management.

We can use a more module-like pattern, rather than putting everything on a Ghost global.. but that doesn't really change anything about how this code works now. Using a classic module pattern, or using a module system like amd and commonjs really add very little at this point.

If we get a lot of client-side dependencies I'd like to use bower. But at the moment we don't have that many. Equally, as the application gets bigger we may go AMD and use require.js but right now, I think all we need to do is solve the build issue, which can be done with something like https://github.com/gruntjs/grunt-contrib-concat

@tgriesser
Copy link
Contributor

The main benefit of a module system is standardization mostly. It makes it easier for everyone to know what's going on, and makes you think about the architecture of how the pieces fit together (ultimately making things easier to test), and then gives you a build tool with r.js to put it all together when cutting a release. That said, the wrapped CommonJS style for AMD is definitely the way to go in terms of code-reuse on the client and server (and readability)...

I've been really liking the format used by when.js, been using it lately when getting knex & bookshelf ready for use with webSQL, etc.

(function(define) {
"use strict"
define(function(require, exports, module) {

// .. your code here

});

})(
  typeof define === 'function' && define.amd ? define : function (factory) { factory(require, exports, module); }
);

@ErisDS
Copy link
Member

ErisDS commented Sep 8, 2013

I've worked with AMD/require.s extensively, it's a fantastic tool and an absolute must for large applications with a web of dependencies.

But we don't have that, we have a small number of dependencies, and the graph is pretty linear. I am aware of what the generalised benefits are but really don't believe that Ghost will benefit from these things at this stage - I really think it is a case of cracking a walnut with a sledge hammer.

However, we really really do need to get a build together which concatenates the files and closes #359, does any one have any ideas other than using require.js?

@jgable
Copy link
Contributor

jgable commented Sep 8, 2013

I've used something called Mincer which is a node implementation of Sprockets from Rails. The syntax for requiring files is really simple for building up a single concatenated file.

For instance an example init.js might be something like:

//= require /shared/lib/jquery.js
//= require /shared/lib/underscore.js
//= require /shared/lib/backbone.js

(function ($, _, Backbone) {
    // TODO: init all the things
}(jQuery, _, Backbone));

It also supports some transpilation type stuff that might prove useful in the future. There is a chance we could also expose something to plugins to allow them to specify their assets and have them concat'd and minified on startup in production.

I could put together a proof of concept on a branch if you think it'd be worthwhile.

@ErisDS
Copy link
Member

ErisDS commented Sep 8, 2013

That would be excellent, seems like a simple solution which suits us right now.

@jgable
Copy link
Contributor

jgable commented Sep 9, 2013

Started on this last night and should have something to show on a branch later tonight.

@ErisDS
Copy link
Member

ErisDS commented Sep 9, 2013

awesome 👍

@thomasboyt
Copy link

Oh hey, this is one of the first things I was wondering about when I started looking into the Ghost source.

I'd love to take a crack at doing this right, if no one else is currently working on it. I work on the ES6 module transpiler, which has stabilized, but may not be the best option for low-friction development (it's not as intuitive as AMD/RequireJS for new users). I would definitely use ES6 compiled to AMD or just vanilla AMD, though; Browserify/CJS is more hassle than it's worth.

One recommendation: it's a good idea to have at least separate vendor and app bundled files, as vendor files change far less frequently than your actually app code. No reason to force mobile users to redownload dependencies unless they're actually changed :)

@ErisDS ErisDS modified the milestones: Ember.js, Multi-user Apr 15, 2014
@ErisDS ErisDS added the client label Apr 15, 2014
@morficus
Copy link
Contributor

morficus commented May 8, 2014

Is anyone currently/actively working on this?
I have extensive experience working with requireJS, so if AMD is the path that was decided I believe that I could start adding value to this task rather quickly :-)

@ErisDS
Copy link
Member

ErisDS commented May 8, 2014

My apologies, this issue should be closed as a result of #2144 and the current project to move the admin to Ember

@ErisDS ErisDS closed this as completed May 8, 2014
@jgable
Copy link
Contributor

jgable commented May 8, 2014

@morficus Just for further clarification, the ember app uses ES6 modules and transpilation during the build process. See the ember-app-kit project for more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:admin Anything relating to Ghost Admin
Projects
None yet
Development

No branches or pull requests

6 participants