Reading/Web/Coding conventions

From MediaWiki.org
< Reading‎ | Web
Jump to navigation Jump to search

This document briefly describes guidelines for writing code for the MobileFrontend MediaWiki extension.

Indenting[edit]

  • Always use tabs for indents.
  • Always indent - even inside JavaScript closures

e.g.

(function() {
  var foo;

Commit Messages[edit]

The commit message should provide enough context for a reviewer to understand the reason for and the reasoning around the change. Therefore, an ideal commit message includes:

  1. A brief statement of problem, which, in the case of stories and bugs, might also include a summary of the discussion on the associated Phabricator task
  2. A high-level description of the solution
  3. If applicable, why the solution was chosen over its alternatives
  4. Issues/concerns with the chosen solution, e.g. performance concerns, which, in the case of the mobile web, are prevalent

Wherever possible, the commit message should include links to supporting information, which will most likely be documentation.

Tagging commit messages[edit]

Subject tags[edit]

  • Hygiene: When tidying up code (e.g. refactoring, addressing FIXME's), prefix your commit with Hygiene: ....
  • QA: Prefix with QA: ... for any patch which updates or builds on our acceptance/browser tests.
  • i18n: Any patch which fixes a localization problem or updates localization messages should commence with i18n: ...

Commit footer labels[edit]

  • Bugs and Stories: If the commit is associated with a Phabricator task, the commit message should finish with Bug: TXXXX where TXXXX is the bug number in Phabricator. There should be no whitespace between this line and the Gerrit Change-ID so that it gets reported to Phabricator by the gerrit bot.
  • Dependencies: When committing a patch with a dependency or dependencies, be sure to include Depends-On: ChangeId where ChangeId is the gerrit Change-Id of the commit that needs to be merged beforehand. When multiple dependencies exist be sure to list them on separate lines. When doing this Jenkins will not allow the merging of your code unless the dependency is present.

Filenames[edit]

File names should use camelCase. In case of PHP files, the name should start with a capital letter and should be named after the main class they contain. If a JavaScript file defines a constructor, then its name should also start with a capital letter. Other files should be started with a lowercase letter.

Do not use the mf- prefix.

PHP test files should be suffixed with Test, JavaScript test files should be prefixed with test_.

Template and Less/CSS files used only by a single class in JavaScript should be named after the class, e.g. TutorialOverlay.less.

Code deprecation[edit]

The code, Api and functions of MobileFrontend can be used by other extensions, which depends on it. Removing existing interfaces or public functions can impair the functionality in these projects. To avoid problems when removing code from MobileFrontend, it's advised to deprecate it before really removing it from the code base. That allows other extensions to still use the existing functions, but get's a warning message, that the code will be removed in a future release of MobileFrontend.

Deprecation in PHP[edit]

There are two things that should be added to code when it is deprecated.

  1. First it needs to be marked as such in the source code with comments (typically, adding @deprecated to the function's Docblock), and all callers should be fixed.
  2. It also should be marked with wfDeprecated(). This will throw errors to any developers still using the code. All callers (in extensions and core) should have since been fixed, but this helps weed out stragglers over the next version. The first argument should be __METHOD__ to use the name of the method in the deprecation. Or more detailed text if you are deprecating just a small chunk of code. The second argument must be the version of MediaWiki you are deprecating the method in. This will allow deprecations to be filtered and also tracked. For example:
/**
* @deprecated since 1.22, use Bar
* @return boolean
*/
public static function Foo() {
	wfDeprecated( __METHOD__, '1.22' );
	return self::Bar();
}

If Foo() is replaced by another function, you should still return the result of the new function to keep Foo() functional with the same call signature and result format.

Deprecation in JavaScript[edit]

Like in PHP, you should document the depreaction of code in JavaScript, too (e.g. in the docblock). Unlike in PHP, deprecation in JavaScript works in another way. Instead of adding a function which will create a deprecation notice, you use mw.log.deprecate() to define a function in an object, which will, when accessed, produce a deprecation warning in the console with a backtrace. E.g., if you want to deprecate function foo() in object bar(), you could do something like:

( function ( mw, $ ) {
	var bar = {
		/**
		 * Do something
		 * @return {String}
		 */
		a: function () {
			// do something
			return 'something';
		},
		/**
		 * Do some other thing
		 * @return {String}
		 */
		b: function () {
			// do some other thing
			return 'some other thing';
		}
	};

	/**
	 * Do foo in bar
	 *
	 * @param {String} AorB Whether a or b should be used.
	 * @return {String}
	 * @deprecated since 1.24 Use a or b directly instead.
	 */
	mw.log.deprecate( bar, 'foo', function ( AorB ) {
		if ( AorB === 'a' ) {
			return 'something';
		}
		return 'some other thing';
	}, 'Use a or b instead' );
}( mediaWiki, jQuery ) );

PHP[edit]

Config variables[edit]

Global config variables set by MobileFrontend need to identify the extension in some way, by general MW convention.

Config variables should be camelCased and prefixed with $wgMF Examples:

  • $wgMFPhotoUploadEndpoint
  • $wgMobileFrontendLogo

Previously '$wgMobileFrontend' was used but was abandoned in favor of '$wgMF...' because of brevity.

ResourceLoaderModules[edit]

Note Note: This is a WIP and the spec is not finalised but the current suggested naming convention is as follows:

When naming a ResourceLoader module in Resources.php, the module name should reflect the functionality it offers and where. For example, the name mobile.special.mobilediff.styles describes a module containing styles that are only applied to the Special:MobileDiff page, mobile.special.mobilediff.scripts describes scripts that should only be applied to the Special:MobileDiff.

Try to group modules by their type:

  • modules related to special pages should start with mobile.special.*
  • modules related to the Minerva skin used by MobileFrontend should start with skins.minerva.*
  • modules implementing a particular feature should start with a word describing it, e.g. mobile.editor.*

There is usually no need include 'beta' or 'alpha' in the module name. Please try to use comments instead to describe where those features reside.

Naming functions[edit]

  • When returning true or false the function should be prefixed with 'is' or 'has' e.g. isBetaGroupMember(), isLoggedIn etc. Functions should be camelCased.

Class names[edit]

ResourceLoader modules[edit]

When naming classes try to avoid the MF prefix and instead try and describe what it does differently from a normal ResourceLoader module. Examples:

  • MobileSiteModule
  • MobileDeviceDetectModule.php

All classes should be written on the basis that one day they might make it into the desktop site. Note: MFResourceLoaderModule needs renaming - it pre-parses messages and provides template rendering.

File organisation[edit]

All classes no matter how small should be in a separate file. This helps with making classes easier to find within the codebase. All files should live in the includes folder (with the exception of global PHP files such as MobileFrontend.php). Remember not every one uses the same IDE as you!

Filenames should be mapped to class names. Burying multiple classes in a file, even if they're small classes, makes discovery a little more difficult for users with basic IDEs. We should encourage discovery of our code to newbies who are not familiar with the codebase.

  • api: Anything related to api goes here
  • modules: Put ResourceLoader module classes here
  • skins: Put skins here
  • specials: put SpecialPage modules here

i18n messages[edit]

If your message contains a single quote/apostrophe, wrap the entire message in double quotes rather than escaping the single quote.

JavaScript[edit]

URL Routing (Use of the URL hash)[edit]

When navigating to a route always prefix it with '/' to distinguish it from element IDs. e.g. #/notifications not #notifications.

File organisation[edit]

Always use camelCase. Group related files in folders describing the functionality.

Naming conventions[edit]

Standard MediaWiki naming conventions apply. Any JavaScript must pass the Manual:Coding_conventions/JavaScript.

Use camelCase for variable names.

When naming events use lowercase letters and a hyphen as the word separator (page-loaded is good, pageLoaded is bad).

Start constructor functions with capital letters.

Use ev as the name of the variable holding event data in event handlers.

Modules[edit]

Each JavaScript file can be a module, i.e. can expose some functionality to other JavaScript files.

A module has one of two purposes

  1. Provide reusable functionality via a class and the use of M.define: A module should only create one class and should use an uppercase character when naming the class e.g. Toast, Api, EditorOverlay. The module may also define an instance of itself via M.define - e.g. M.define( 'api' ) = new Api()
  2. Execute some code providing some new functionality: This tends to use classes defined in other modules.

When writing modules be sure to wrap each module (in fact, every JavaScript file) in a closure.

Use closure's arguments to alias mw.mobileFrontend and jQuery objects as M and $ respectively (but only if the module needs them):

( function( M, $ ) {
    // module contents
}( mw.mobileFrontend, jQuery ) );

Expose module's functionality using M.define:

( function( M, $ ) {
    // module contents
 
    M.define( 'moduleName', {
        SomeConstructor: SomeConstructor,
        someFunction: someFunction
    } );
}( mw.mobileFrontend, jQuery ) );

Module's name should be the same as module's file name (without the .js extension).

If the only thing exposed by a module is a class/constructor function, then the module name (and file name) should be capitalized:

( function( M, $ ) {
    // module contents

    M.define( 'modules/uploads/Uploader', Uploader );
}( mw.mobileFrontend, jQuery ) );

Use other module's functionality using M.require:

( function( M, $ ) {
    var someFunction = M.require( 'moduleName' ).someFunction;

    // module contents
}( mw.mobileFrontend, jQuery ) );

jQuery[edit]

Use $( '<div>' ) rather than $( '<div/>' ) when creating new DOM nodes (or even better, use templates).

Use on to bind events rather than other convenience functions such as click

Use jQuery.Deferred as a return value of asynchronous functions. Use #resolve/#done for success and #reject/#fail for errors.

Use jQuery objects in favor of native DOM elements (for the sake of consistency).

Avoid using $( document ).ready unless necessary. Most of the JavaScript files are loaded at the bottom of the page anyway.

Avoid jQuery/HTML spaghetti code, instead use View and Hogan templates (same syntax as Mustache).

Views[edit]

When dealing with JavaScript views, have a look at Mobile_web/Coding_conventions/JavaScript/Views, you will find information and good practices.

Transpiling[edit]

We use transpiling in code repositories such as Extension:Popups Given that ES6 template literals provide similar readability to template and are part of JavaScript itself, we consider this to be a favorable and sustainable alternative to Mustache templates where available. Additionally, although the usage of template strings requires transpilation, adding transpiling support enables other ES6 syntaxes to be used such as let / const, arrow functions, and destructuring, all of which are considered language improvements that Extension:Popups can leverage in many areas.

When transpiling, extra care is necessary in code review to pay heed to browser support for native functions e.g. Object.assign; Array.prototype.include; given using modern JavaScript gives the false illusion that browser support is relatively modern. It is not feasible to use a linter to check for forbidden methods (see phab:T190104) so take care!

CSS/Less[edit]

We currently use Less to generate stylesheets. You need Node.js for the Less compiler to run.

Ensure all CSS passes the Manual:Coding_conventions/CSS.

Do not use the mw-mf- prefix.

Use lowercase letters and a hyphen as the word separator in class and id names (search-box is good, searchBox is bad).

Avoid using attribute selectors - these are known to cause issues in the phonegap app which shares some of this codebase.

Colors[edit]

Reuse colors in variables.less file. Do not introduce new colours outside this variable file, especially when you work with the color gray.

Icons[edit]

Reuse the icon classes in icons.less When introducing a new icon, only put it in icons.less if the icon is widely used. If not, be sure that the class name that serves it is prepended with 'icon-'

Units[edit]

When using values less than 1 write without the leading 0 e.g. .5em not 0.5em, .37em not 0.37em

Use shorthand where possible[edit]

Where ever possible use the shorthand rule to reduce number of CSS rules. As a rule of thumb if you have a margin-left and a margin-top in the same rule you should probably be using margin. Use background rather than background-image where possible.

HTML[edit]

HTML should validate via W3C validator. Note, at the current time do not worry about markup introduced via wikitext and the 1 validation error that occurs due to MediaWiki core.

Tests[edit]

QUnit[edit]

  • All QUnit tests should be prefixed with 'test_'
  • Tests should mirror the directory structure of the thing they are testing

e.g. javascripts/modules/foo.js should have a test as tests/qunit/modules/test_foo.js

  • QUnit modules should be named after the JavaScript module they are testing and prefixed with MobileFrontend

e.g.

QUnit.module( 'MobileFrontend modules/editor/EditorApi' )

PHPUnit[edit]

  • All PHPUnit tests should be suffixed with '_test'

Browser tests[edit]

Please look at the serious guidelines for writing browser tests.


Steps should be written in sentence case. e.g.

  • Given the user is logged in
  • When the user hits the button
  • Then I see the unicorn
    • And the unicorn pukes a rainbow at me
    • And William McKinley's ghost appears
    • And there is lightning

When a step has arguments wrap it in quotes e.g.

  • Given I enter username "Foo"
    • And I type my password
    • When I click submit
    • Then I see a toast message with the text "Welcome!"