Skip to content

Latest commit

 

History

History
301 lines (227 loc) · 13.1 KB

GUIDELINES.md

File metadata and controls

301 lines (227 loc) · 13.1 KB

Projects Coding & Testing Guidelines

This guide covers the coding guidelines common to all ibm-js hosted projects.

End User Requirements

  • All widget/custom elements MUST be enabled for globalization (including translatability & bidi support)
  • All widget/custom elements MUST be enabled for accessibility:
    • keyboard support
    • zoom (full zoom, not text only zoom, up to 200%)
    • screen reader accessibility (supported readers are JAWS on Windows and VoiceOver on iOS)
    • must support high contrast (IE and FF on Windows, with high contrast mode enabled in the OS settings); device specific themes (currently iOS and holodark) are exempt from this rule
  • The projects will only support modern browser/platforms: FF31+, Chrome latest, IE9/10+, Safari 7+, Android 4.1+, iOS6+, WindowsPhone8+

See https://docs.google.com/document/d/1Sa0rn4udqO_ea20o4xxogRP6P6ZLgHPMEj4A2V4g7hM for more details.

Javascript Coding Guidelines

Our components are written in Javascript rather than a derivative like Typescript or CoffeeScript.

JSHint

All projects MUST comply with the directives in this jshintrc and MUST enforce it through a TravisCI check.

JSHint max complexity is set to 10 by default but CAN be temporarily put up to 15 for a given method if the committer feels this is not hurting the code readability.

AMD

For projects to be used in the browser (as opposed to Node.js specific projects), modules are wrapped in AMD rather than CommonJS or UMD format. Rationale: this is mainly for the benefit of the build tool.

RequireJS is our main AMD loader. Plugins and builds are tailored for RequireJS.

We standardize on using the following plugins:

API documentation

Public methods and properties should use JSDoc format, as detailed in https://docs.google.com/document/d/1fGBLuDAJRHFuSE4_NM2YfbRcCeYV40KWDXCuVyiSV_U/edit#heading=h.mrkbtrfyd36f

Other JavaScript Coding Guidelines

  • Getters/Setters MUST use ES5 getter/setter and not getProp/setProp methods.
  • Calling mycomponent.myproperty = value for ES5-like setters SHOULD refresh the component either directly or using a simple delayed refresh mechanism provided by decor/Invalidating. If that is not the case it must be clearly documented
  • Variables SHOULD be declared as close to the point where they are first used as it helps to cluster groups of code together instead of spreading them throughout a function. Variables that are assigned in multiple places of a function (e.g. loop counters) CAN be declared at the beginning of the function.
  • In order to associate an object to be used as this when calling a function:
    • If the called function is public and documented the code MUST use a closure construct with a self variable to preserve AOP:
    var self = this;
    this.on("DOMNodeInserted",  function(){ self.publicmethod(); });
    • If the called function is either private or not documented the core MUST use Function.prototype.bind():
    this.on("DOMNodeInserted",  this._privatemethod.bind(this));
  • By default, code SHOULD NOT check properties or parameters for validity, if there's a particular case where it's especially helpful to the user to check, then code SHOULD:
    • throw an exception rather than using console.log() or console.error()
    • add comment to the code about why you throwing the exception
  • Widget Template MUST have a top level tag of type <template>.

OO

If you need an OO framework to declare classes (with multiple inheritance, advice, etc.), we use dcl. However, it's also acceptable to declare classes native by specifying a constructor and prototype.

For classes defined by dcl (or delite/register), a method in a subclass should call the superclass' method (of the same name) via dcl.superCall(), or its alias delite/register#superCall(). For example:

appendChild: dcl.superCall(function (sup) {
	return function (child) {
		var res = sup.call(this.containerNode, child);
		this.onAddChild(child);
	};
})

dcl.after()/dcl.before()/dcl.around() should be reserved for when the superclass needs to execute some code before/after all the subclasses' methods run. For example, delite/Widget#attachedCallback() sets this.attached = true after the attachedCallback() methods in all the subclasses run.

Note that we sometimes automatically chain class methods using dcl.chainAfter() / dcl.chainBefore(), for example delite/Widget does:

dcl.chainAfter(Widget, "postRender");

In this case, the dcl.superCall() is implied, and should not done explicitly, for example, a widget's postRender() method would be:

postRender: function () {
	// code here will be executed after superclasses' postRender() methods run
}

Inheritance Guidelines

  • Visual components (aka widgets) MUST extend delite/Widget
  • Non visual components SHOULD extend decor/Stateful

Libraries

We've standardized on a number of support libraries, both in and outside of the ibm-js account:

  • has - Feature and browser sniffing is done via has.add() and has(), so that a build can strip unneeded code. We use the has() implementation in ibm-js/requirejs-dplugins/has.

See also the "AMD" and "OO" sections above.

Properties/functions naming

  • Properties and functions that are not meant to be used by the application developer (either private or only used internally) MUST either:
    • be closure-bound in the AMD module without underscore
    • start with underscore
  • Mixin private methods CAN be postfixed or marked by the Mixin name to avoid potential clash

Properties naming

  • Properties having similar effects on components MUST share the same naming. Examples:
    • property to set/get data store MUST be called store
    • property to set/get selected items MUST be called selectedItems
  • Properties that are used to bind a component property to a data for a property xxx MUST be called (StoreMap mixin is enforcing that in delite):
    • xxxAttr for binding by data attribute (like labelAttr for label property)
    • xxxFunc for binding by data function (like labelFunc for label property)

Events naming

  • component/mixin-specific events SHOULD be prefixed by the component/mixin AMD module name, e.g. button-whatever, selection-change etc..
  • project-specific events that can be used cross-components MUST be prefixed by the project name e.g. dapp-eventname, delite-display...
  • event topic names MUST be namespaced by the project name e.g.delite/topicname
  • event handlers passed to on, addEventListener SHOULD be named eventNameHandler or _eventNameHandler depending on whether they are public or private.

Class/Modules naming

  • Modules that return an instantiable object MUST start with capital letter
  • Modules that return an object that can’t be used as a constructor MUST start with lowercase letter
  • Modules whose interface (aka methods) is not used by the application developer (either because it is fully private or because it is only used by component developer) MUST start with underscore. That means totally private classes but also mixins on public classes that are here for purely internal reasons.
  • Modules whose interface (aka methods) is used by the application developer MUST NOT start with underscore even if the application developer does not instantiate it. That means any class or mixin that has at least a property or method the application developer needs to call.
  • Public classes and that must not be instantiated because they are just parent classes for implementations SHOULD be suffixed by Base (e.g. CalendarBase) only when they need to differentiate themselves from the actual user accessible implementation (e.g. Calendar).
  • Mixins SHOULD NOT be postfixed by Mixin (ala dgrid or dtreemap, so Selection not SelectionMixin for example)

Security Guidelines

Use of possible evil functions like eval() or equivalents SHOULD be avoided as much as possible.

When they cannot be avoided, each use of an evil function MUST be hinted by jshint in the code.

The code MUST also contain a comment explaining why we need that call, when it will be triggered. From this we will be able to extract a security guide listing all the potentially harmful call and recommendation for the user.

CSS

  • CSS is generated from Less files (i.e. we are using Less rather than Stylus)
  • JavaScript code SHOULD NOT hard code style but rely on stylesheets
  • !important MUST NOT be used
  • Stylesheets MUST clearly document which properties we don't support changing. It SHOULD layout properties and rules to clearly separate what is not supposed to be changed.
  • Widgets styling SHOULD NOT use float style property.
  • Widgets Samples SHOULD use delite/themes/defaultapp.css when:
    • laying out element with borders (defaultapp.css set border-box on everything)
    • using an element that uses the whole height of the page (height:100% is needed on ascendants, including body)
    • in need of getting rid of the default margin/padding defined by browsers
  • bootstrap theme will use font-icons rather than CSS sprites, in order to support high-contrast mode and also theme customization (i.e. changing the colors of the icons)

CSS class naming

  • component specific CSS classes MUST be prefixed by the component tag name, e.g. d-button-whatever, d-treemap-whatever etc..
  • cross-components CSS classes MUST be prefixed by d-, e.g. d-selected etc… and shared across the various projects / components.
  • private CSS classes MUST be prefixed by “-” and follow the practices above (so -d-button-whatever)

Testing Guidelines

API For Writing Tests

All tests MUST be written using Intern. Tests should be written using intern!object, which provides the registerSuite() method and also the API for functional tests.

For assertions, we use the intern/chai!assert module. We prefer assert.strictEqual() to assert.equal(), just like we prefer === to ==. Also, we add hints to each assertion so if there's a failure we know which assert fired. For example:

assert.strictEqual(width, "100px", "width");
assert.strictEqual(height, "100px", "height");

Functional tests

For functional tests, we are using the Intern Promised Based Webdriver API, where the test is written as a promise chain, and that chain is returned, ex:

return this.remote
	.elementById("stub-for-blurring")
		.click()
		.end()
	.wait(500)
	.elementById("choiceDropDown")
		.isDisplayed(function (err, displayed) {
			assert.isFalse(displayed, "choiceDropDown popup not visible");
		})
		.end();

Unit tests

When practical, asynchronous unit tests should be written as chained promises too, for example:

this.timeout = 10000;		// (if necessary; default is 1000ms)
return myWidget.open().then(function () {
    return myWidget.close();
}.then(function() {
    var cs = getComputedStyle(myWidget);
    assert.strictEqual(cs.width, "100px", "myWidget width");
});

When it's not possible to make a promise chain, we use Intern's this.async(timeout) method, along with d.rejectOnError() and d.callback(), for example:

var dfd = this.async(10000);
myWidget.open();
setTimeout(d.rejectOnError(function () {
   myWidget.close();
   setTimeout(d.callback(function () {
     var cs = getComputedStyle(myWidget);
     assert.strictEqual(cs.width, "0px", "myWidget width");
   }, 100);
}), 100);

Note how the final asynchronous callback must be wrapped by d.callback(), and all other (intermediate) asynchronous callbacks are wrapped by d.rejectOnError().

Widget Testing

Unit tests SHOULD cover

  • checking the widget state including default properties values after programmatic creation
  • checking the widget state including default properties values after markup creation
  • setting & checking dynamically each property (except the ones clearly documented as not being mutable) on a created instance
  • calling & checking each widget public method on a created instance
  • checking that each event the widget is firing on method calls / property change is correctly received
  • checking the widget resources are properly released on destroy

Functional tests SHOULD cover

  • checking the state of the widget during and after the typical user interaction scenarios for the widget
  • checking that each event the widget is firing on user interaction is correctly received