Gaia/System/Refactoring Plan: Difference between revisions

From MozillaWiki
< Gaia‎ | System
Jump to navigation Jump to search
 
(15 intermediate revisions by 3 users not shown)
Line 13: Line 13:
=== Stage.1 - Painless instantiation ===
=== Stage.1 - Painless instantiation ===


Rewrite all modules to be instantiable and rewrite unit tests, as well as jsdoc. '''Everything that has states need to be an instance, even if we only need one in the app.'''
'''Everything that has states need to be an instance, even if we only need one in the app.''' Rewrite all modules to be instantiable and rewrite unit tests, as well as JSDOC.


The stage’s target is to make sure we could have the following pattern:
See [[Gaia/System/Refactoring_Plan#Stage_1_example|Stage 1 example]] for detail.


bootstrap.js (starting point, simply one-liner)
There are already bugs (87 bugs, excluding downloadmananger and fxaccounts for now. Take as you like!) of this stage for every js under the system app. See meta bug.
 
<pre>
 
// (we need to expose the system instance because test atoms need to call some functions directly)
window.system = new System();
window.system.start();


</pre>
==== Initialization ====


system.js (the "root" of the all modules, starts the "world")
'''We should not change where we initialize the instance of the module in this stage.''' We do this in Stage 1 to ensure we don't don't need to resolve the launching sequence puzzle for now. If the module your are dealing with simply start itself, feel free to add


<pre>
<pre>


var System = function System() {
   /**
   /* some initial states */
  * Start ourselves
  this.started = false;
  * XXX: To be moved.
};
  */
System.prototype.start = function() {
   window.foo = new Foo();
  this.started = true;
   window.foo.start();
 
  /* modules */
   this.appWindowFactory = new AppWindowFactory(this);
   this.appWindowManager = new AppWindowManager(this);
  this.screenManager = new ScreenManager(this);
 
  /* event listeners */
};
 
.. other prototype methods follows ..


</pre>
</pre>


'''(TBD: jsdocs)'''
at the bottom of the file. If the module is initialized elsewhere, e.g. <code>bootstrap.js</code>, you can still safely do that there.
 
'''Caveat''': Please take care of instantiation timing and dependency. Please ensure the rewrite does not break anything (run tests!). A pair of <code>init()</code>/<code>start()</code> and <code>uninit()</code>/<code>stop()</code> function is recommended to give the instance an explicit start and finish.
 
With this pattern we are able to do unit test anywhere,
and there's no architecture design involved in this stage.
Even a new employee could take one of the modules to rewrite it correctly.


There are already bugs (87 bugs, excluding downloadmananger and fxaccounts for now. Take as you like!) of this stage for every js under the system app. See meta bug.
'''Note'''
* You are required to resolve jshint errors in this stage and remove the file from blacklist.
* You are required to have unit tests in this stage.
* If you register any event listener in start(), '''remember to remove them in stop()'''. Otherwise, our unit tests will get confused because we run all unit tests in the same iframe and module dependencies will mess things up. For the self-initialized script, do call the <code>stop()</code> method on the "global" instance in the test <code>setup()</code>.


=== Stage.2 - Architecture love ===
=== Stage.2 - Architecture love ===
==== Find out loading sequence and dependency relationships ====
==== Find out loading sequence and dependency relationships ====
1. File bugs to track all dependency resolving.
==== Do architecture review and rework case by case ====
==== Do architecture review and rework case by case ====
# Find the unnecessary module coupling and remove it.
# Find the unnecessary module coupling and remove it.
Line 96: Line 79:
and jsdoc will be generated to 'docs' folder
and jsdoc will be generated to 'docs' folder


=== Sample ===
=== Stage 1 example ===
<pre>
<pre>
'use strict';
'use strict';
Line 116: Line 99:
     */
     */
     show: function(data) {
     show: function(data) {
      /**
        * DESCRIPTION-OF-THE-EVENT.
        * @event Module#i-am-displayed
        */
      this.publish('i-am-displayed');
    },
    /**
    * DESCRIPTION-OF-WHAT-START-DO
    * @memberof Module.prototype
    */
    start: function() {
      if (this._started) {
        throw 'Module XXX have already started.';
      }
      this._started = true;
      window.addEventListener('someevent', this);
    },
    /**
    * DESCRIPTION-OF-WHAT-STOP-DO
    * @memberof Module.prototype
    */
    stop: function() {
      if (!this._started) {
        throw 'Module XXX have not started.';
      }
      this._started = false;
      window.removeEventListener('someevent', this);
    },
    /**
    * DESCRIPTION-OF-EVENT-HANDLER
    * @memberof Module.prototype
    */
    handleEvent: function(evt) {
     },
     },


     /**
     /**
     * DESCRIPTION-OF-IMPORTANT-PUBLIC-ATTRIBUTE
     * DESCRIPTION-OF-IMPORTANT-PUBLIC-ATTRIBUTE
     * @memberof DevtoolsView.prototype
     * @memberof Module.prototype
     */
     */
     state: 'begin'
     state: 'begin'
Line 129: Line 148:
}(window));
}(window));
</pre>
</pre>
[[File:jsdoc-sample.png|300px|thumb|left|Output with default theme]]

Latest revision as of 13:34, 22 February 2014

Links

  1. [META] http://bugzil.la/system-app-v2
  2. [JSDOC] http://alivedise.github.io/gaia-system-jsdoc

Target

The main purposes/targets are:

  1. Minimize the memory usage of the system app
  2. More maintainable and testable
  3. Easier for a new contributor to jump in
  4. Scalable on multiple targets (TV, phone, tablet, ...)

Stages

Stage.1 - Painless instantiation

Everything that has states need to be an instance, even if we only need one in the app. Rewrite all modules to be instantiable and rewrite unit tests, as well as JSDOC.

See Stage 1 example for detail.

There are already bugs (87 bugs, excluding downloadmananger and fxaccounts for now. Take as you like!) of this stage for every js under the system app. See meta bug.

Initialization

We should not change where we initialize the instance of the module in this stage. We do this in Stage 1 to ensure we don't don't need to resolve the launching sequence puzzle for now. If the module your are dealing with simply start itself, feel free to add


  /**
   * Start ourselves
   * XXX: To be moved.
   */
  window.foo = new Foo();
  window.foo.start();

at the bottom of the file. If the module is initialized elsewhere, e.g. bootstrap.js, you can still safely do that there.

Note

  • You are required to resolve jshint errors in this stage and remove the file from blacklist.
  • You are required to have unit tests in this stage.
  • If you register any event listener in start(), remember to remove them in stop(). Otherwise, our unit tests will get confused because we run all unit tests in the same iframe and module dependencies will mess things up. For the self-initialized script, do call the stop() method on the "global" instance in the test setup().

Stage.2 - Architecture love

Find out loading sequence and dependency relationships

1. File bugs to track all dependency resolving.

Do architecture review and rework case by case

  1. Find the unnecessary module coupling and remove it.
  2. Split a single "heavy-loading" module into different modules with its specific responsibility
    1. For instance, split window manager into 3 pieces: manager, factory, and classes.
  3. Move the static DOM elements into its responsible module and let the module render its own UI with whatever template engine.
  4. (optional) Use event emitter to replace DOM elements. (But we need to find out 'home' event replacement.)
  5. Find out patterns and figure out if we could group them.
    1. For example many modules are depending on a specific settings to be ON/OFF. Maybe we could lazy load them in a SettingsOberver.

Some candidate bugs

  1. Clean widget-like cost control
  2. Clean rocketbar
  3. Manage hardware dependent modules together by feature detection
  4. Split lockscreen
  5. TrustedUI rework
  6. Keyboard management rework
  7. FxaAccount review
  8. DownloadManager review
  9. CardsView rework

Stage.3 - Do experiments

  1. We could start trying to load everything inside iframe as Vivien's lovely-proposal because we had the similar interfaces for modules now.
  2. We could introduce module loader solution in this stage. I personally isn't the fan of any of them so I don't insist on this item is must-have.
  3. Work out a clear proposal for system app for different kind of target devices.
  4. Do whatever you want - system should be stable enough now!

FAQ

How to run jsdoc locally?

npm install
grunt docs

and jsdoc will be generated to 'docs' folder

Stage 1 example

'use strict';

(function(exports) {

  /**
   * DESCRIPTION-OF-THIS-MODULE.
   * @class Module
   */
  function Module() {
  }

  Module.prototype = {

    /**
     * DESCRIPTION-OF-IMPORTANT-PUBLIC-API
     * @memberof Module.prototype
     */
    show: function(data) {
      /**
        * DESCRIPTION-OF-THE-EVENT.
        * @event Module#i-am-displayed
        */
       this.publish('i-am-displayed');
    },

    /**
     * DESCRIPTION-OF-WHAT-START-DO
     * @memberof Module.prototype
     */
    start: function() {
      if (this._started) {
        throw 'Module XXX have already started.';
      }
      this._started = true;
      window.addEventListener('someevent', this);
    },

    /**
     * DESCRIPTION-OF-WHAT-STOP-DO
     * @memberof Module.prototype
     */
    stop: function() {
      if (!this._started) {
        throw 'Module XXX have not started.';
      }
      this._started = false;
      window.removeEventListener('someevent', this);
    },

    /**
     * DESCRIPTION-OF-EVENT-HANDLER
     * @memberof Module.prototype
     */
    handleEvent: function(evt) {
    },

    /**
     * DESCRIPTION-OF-IMPORTANT-PUBLIC-ATTRIBUTE
     * @memberof Module.prototype
     */
    state: 'begin'
  };

  exports.Module = Module;

}(window));
Output with default theme