MobileFrontend/JS code organization

Current state

Currently, JavaScript code in MobileFrontend is divided into separate modules (files). Each of them is responsible for a distinct feature in the mobile frontend, e.g. modules/mf-toggle-dynamic.js folds and unfolds sections in articles, specials/watchlist.js adds additional feature to the mobile watchlist (thumbnails). The concept is similar to plugins.

Those modules are usually subsequent improvements added to the mobile frontend, not necessarily logical blocks of the application. Each module is optional and provides a javascript enhancement on top of the normal view.

Modules are sandboxed and should not communicate with one another. Where common functionality is identified historically this has been moved to mf-application.js. A module may bind triggers to certain events to allow other modules to run on these.

Some modules use the mw.mobileFrontend.registerModule() function which expects an object with an init() function as its argument. After the main code is initialized the init() function of each module is called. This is used by modules which may need to reinitialize given a change of context - for example every time a page is dynamically loaded.

Other modules simply do something without ever calling mw.mobileFrontend.registerModule(), which can be binding to an event of the window object or performing some actions after the DOM is ready. This method is only used by modules which do not need to run more than once.

There are no guidelines as to how modules should look like. They are usually a collection of functions that perform everything needed for the given module without following any programming paradigms. Object oriented patterns are not used, there are no separate objects which would take care of different parts of a particular feature, like interpreting data from APIs (model) and building user interface (controllers/views/templates).

Very often a single JavaScript function has multiple responsibilities, like fetching data from API, fetching data from DOM, manipulating this data and rendering the user interface.

Problems

There are two main problems with our current JavaScript code:

Lack of code modularity, lots of functions that "do everything" (this is not about splitting code among many files, it's about enforcing Separation of concerns).
- I'm not sure I agree... Can you give an example? I think most modules are large as they introduce new functionality that other modules have no need for. Over time these have been split into common functions in javascripts/common/ directory (e.g. notifications for example) Jdlrobson (talk) 20:45, 10 January 2013 (UTC)[reply]
  - I was refering to long functions rather than long modules here. Let's take init() from watchlist.js as an example. It fetches data from DOM (L17), fetches data from API (L24), and modifies the UI (L37). In the meanwhile it also registers event triggering on L21 and it is unclear what is the purpose of this event without grep-ing all the code (and even after grep-ing the name "mw-mf-watchlist" appears also as a CSS id and I have to find the right occurences manually). JGonera (WMF) (talk) 04:03, 15 January 2013 (UTC)[reply]

Lack of a standardized module system (just loose JS files).
- Does desktop have one? As far as I know it to just makes use of loose JS files Jdlrobson (talk) 20:45, 10 January 2013 (UTC)[reply]
  - I don't know if desktop has one, but desktop is a much less dynamic web site than the mobile one, and much older too. JGonera (WMF) (talk) 04:03, 15 January 2013 (UTC)[reply]

Code modularity

The lack of code modularity has four consequences:

It makes code reuse or even noticing that a part of the code could be reused harder.
- Not sure I agree here. Common functionality can be found on mw.mobileFrontend or mw Jdlrobson (talk) 20:45, 10 January 2013 (UTC)[reply]
  - I agree, common things are in mw.mobileFrontend, but only those that were easy to extract. An example of a common functionality that comes to my mind but is missing is some kind of a wrapper around the API requests ("model" from MVC?). We always use $.ajax() but half of the parameters don't change (dataType and format never change, URL is often the same), we always concatenate lists manually to send them in the format that APIs understand (titles.join( '|' ) instead of just titles). Those might seem like small details but they add up and make the code look less clean, they add unnecessary noise. JGonera (WMF) (talk) 04:03, 15 January 2013 (UTC)[reply]

It makes reviewing the code more tedious. When even a small change is introduced in a large function that has many responsibilities, the reviewer often has to reread the whole function to understand that small change.
- Example of a large function that is difficult to code review? Jdlrobson (talk) 20:45, 10 January 2013 (UTC)[reply]
  - Most in mf-photo.js. They're just very difficult to follow for me. Might be not only the length of the functions but also the fact that we don't use OO programming in general (no objects that represent separate concepts and have their own methods). I'd like someone else to comment on that too. Maybe it's more about my personal habits, but I see OO as a good way of organising code and making it more understandable. Objects help to link the code with application concepts and behavior. JGonera (WMF) (talk) 04:18, 15 January 2013 (UTC)[reply]
- I agree with Jgonera - keeping functions restricted to doing one thing (as opposed to a bagillion, unless it's some sort of controller function) makes it very difficult to quickly understand what's happening in the function. There are many good examples of the benefits of single-purpose functions in the PHP side (particularly in MobileContext.php) and many examples of nasty monolithic multi-purpose functions in the PHP side as well (particularly in the Skin* files). Hand the skin files to a new developer or someone who hasn't touched them in a long time and watch their eyes bleed. Hand MobileContext to a new developer or smoeone who hasn't touched it in a while, and they will squee with delight at how easy it is to understand. Arthur Richards (talk) 19:52, 15 January 2013 (UTC)[reply]
- Further, the JS codebase at the moment is severely lacking in comments, which makes it insanely difficult to follow. Increasing modularity and creating more single-purpose functions can help with this when comments/documentation is lacking (so long as function names are descriptive enough). Arthur Richards (talk) 19:54, 15 January 2013 (UTC)[reply]

It makes testing more difficult. A piece of code that has many responsibilities can not be unit-tested which often leads to not testing that code at all.

It makes the code base more brittle. This is true in part because it makes testing more difficult - but also because it makes it easier for small changes in monolithic functions to cause breakage, which can sometimes obscured. Breakage can be easily obscured when code is written like this when it is non-obvious on user-facing features, or more commonly, because it is difficult to notice in a function that does a bagillion different things

TODO: add example of tangled code and how it can be untangled (modularized)

Module system

The lack of a good module systems has the following consequences:

Modules do not inter-operate, i.e. one module can't use another module's functionality easily. Right now we put all the functions we need in several places in mf-application.js but this file keeps growing and the functions it contains are often not related (e.g. supportsPositionFixed() and getToken()).
- Agreed - mf-application.js could and should definitely be reorganised. Note I expect things like getToken can eventually be replaced by functions in the mw library. Modules should not be able to inter-operate and where they can should make use of events - we should work on the assumption that any module can be removed without breaking any other module. Jdlrobson (talk) 20:45, 10 January 2013 (UTC)[reply]
  - I agree that modules as they are now, that is one module-file = one functionality, shouldn't inter-operate in most cases. I was thinking about introducing more fine-grained modules, namely being able to split what we call now a module into a few separate JS files. It's a common pattern of having a single class/prototype definition per file. If we used object oriented programming in our code, there would be no way of having different classess/prototypes in different files because they wouldn't live inside the same closure and wouldn't be able to see each other. I like how RequireJS solves this, but we might simply set some guidelines of how different files should register their stuff inside mw.mobileFrontend if they need to and assume that no file will try to use other file's stuff before DOM is ready. JGonera (WMF) (talk) 04:41, 15 January 2013 (UTC)[reply]

There is no dependency management (there is no way to indicate that a module requires any other module to be loaded before it can run, we have to manually figure out loading order of JS files in PHP files).
- Modules shouldn't depend on other modules. Modules might made use of additional libraries - e.g. jQuery.msg for example but there IS dependence management in that these dependencies can be managed in ResourceLoader. Jdlrobson (talk) 20:45, 10 January 2013 (UTC)[reply]
  - You're right. I keep forgetting about dependencies in ResourceLoader. JGonera (WMF) (talk) 04:41, 15 January 2013 (UTC)[reply]

Solutions

Developers struggled with JavaScript code organization in many other big projects before us, so the most obvious solution to make the code more modular would be to use one of the popular JavaScript frameworks to structure the code. Apart from that we need to standardize our modules somehow.

Frameworks

There are two important things to note where it comes to our mobile frontend though if we are to use a framework:

We use various data sources. Typically, web apps rely on a standardized AJAX API to get data. We have many APIs that sometimes don't follow guidelines and are not RESTful. We also often fetch required data from the DOM of the server-side generated web site (e.g. titles of articles in the watchlist to query the PageImages extension). This means we need really flexible models.
Our web app is not a typical one-page web app where everything is loaded dynamically. It's more of a hybrid between a web site and a web app. The main reason for this is compatibility with devices with no JavaScript support. In the future we might think about separating it into a legacy mode without JS and a one-page dynamic web app. This would make the development easier and the mobile fronted more responsive.

The framework we choose should satisfy the following:

Be popular, with active community and good docs.
Be small (mobile).
Be flexible, do not impose too much on how we communicate with the backed or how we modify DOM.
Do not assume a fully dynamic one-page app.
Should provide only the barebones functionality we need. TODO:What is this? Jdlrobson (talk) 20:47, 10 January 2013 (UTC)[reply]

Name	Pros	Cons
Backbone.js	Very popular Small Flexible enough (overriding `Backbone.sync()`)	Assumes REST API by default (but can be overridden)
Knockout	Very popular Seems to do more for you than Backbone	Not that small (but still acceptable) Lots of custom magic (`ko.observable, ko.computed`), learning curve might be steeper, maybe less flexible
Sammy.js

Modules

Right now modules are just scripts that do something by themselves and optionally have an init() function. It would be useful to have something similar to AMD modules. We should investigate if Resource Loader can help us with that.