Reading/Web/Coding conventions
This document briefly describes guidelines for writing code for projects owned by the Web team. In general, we strive for compliance with Manual:Coding conventions . However, additions and deviations from these conventions are noted here.
Design principles
[edit]- Inheritance should be avoided. Favor composition over inheritance and use interfaces (in PHP) where polymorphism is needed. New PHP classes should use the
final
keyword by default to discourage misuse internally and by third-parties. - Classes should be kept small and their purpose should be well defined. Avoid cramming too many responsibilities into one class. It might especially be tempting to do this in classes that have generic sounding names (e.g SkinMinerva.php), but don't do it! It's much easier to understand and change classes that follow the single responsibility principle.
- Write your best and help others become better programmers. Patches should be small as is practical, deliberate, and meticulous. For paid contributors (staff), they should be no less than professional and set a good example.
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:
- 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
- A high-level description of the solution
- If applicable, why the solution was chosen over its alternatives
- 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]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 GerritChange-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
whereChangeId
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 suffixed with .test
e.g. Overlay.test.js
.
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 some of our projects such as MobileFrontend can be depended upon by other extensions. Removing existing interfaces or public functions can impair the functionality in these projects. To avoid problems when removing code from these types of projects, 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 a warning message that the code will be removed in a future release.
When deprecating or removing there is no need to update any release notes unlike in core.
Deprecation in PHP
[edit]There are two things that should be added to code when it is deprecated.
- 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. - 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]We adhere to Manual:Coding conventions
Config variables
[edit]Global config variables set by the extension or skin, need to identify themselves in some way, by general MW convention.
Config variables should be camelCased and prefixed with $wgExtensionName (or an abbreviated version e.g. MF for MobileFrontend) or $wgVector (for skins)
Examples:
- $wgMFPhotoUploadEndpoint
- $wgMobileFrontendLogo
Previously '$wgMobileFrontend' was used but was abandoned in favor of '$wgMF...' because of brevity.
ResourceLoaderModules
[edit]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.
Mustache
[edit]Mustache properties must be typed at the top of each Mustache file. For example, at the time of this writing, the VectorTabs.mustache file looks like:
{{!
string tabs-id
string|null empty-portlet
string label-id
string|null msg-label
string|null html-userlangattributes
string|null html-items
}}
<div id="{{tabs-id}}" role="navigation" class="vectorTabs {{empty-portlet}}" aria-labelledby="{{label-id}}">
<h3 id="{{label-id}}">{{msg-label}}</h3>
<ul {{{html-userlangattributes}}}>
{{{html-items}}}
</ul>
</div>
JavaScript
[edit]Comments and documentation
[edit]Function input and output types must be documented with JSDoc syntax.
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 the resources folder 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
- 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()
- 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
or jQuery.Promise
as a return value of asynchronous functions. Use
#then
/#catch
instead of #done
/#fail
/#always
as only the former is ES6 Promise compatible.
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).
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]Ensure all CSS passes the Manual:Coding_conventions/CSS.
Naming Conventions
[edit]The current convention for Vector styles follows a BEM-like structure wherein the following naming pattern is used:
// quasi-BEM
block-element-modifier
.block {}
.block-element {}
.block-modifier {}
vs.
// bonafide BEM
block__element--modifier
.block {}
.block__element {}
.block--modifier {}
Use lowercase letters and a hyphen as the word separator in class and
id names (search-box
is good, searchBox
is bad, searchbox
is bad either for non-English native devs).
The decision not to adhere to a strict BEM approach at this time was informed ultimately by the difficulty in enforcing BEM structure through linting rules.
Lengthy discussion about naming conventions/methodologies continues to evolve and as guidelines are agreed upon (i.e. flattening grandchildren, capping class names, etc), the class naming conventions will be updated accordingly.
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]Use ResourceLoaderOOUIIconPackModule for generating icon definitions. Do not ship SVGs 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]Frontend and backend changes should be accompanied by unit tests where practical.
QUnit
[edit]- All QUnit tests should be suffixed with '.test' e.g. 'foo.js' should be tested by 'foo.test.js'
- Tests should mirror the directory structure of the thing they are testing
e.g. resources/module.name/foo.js should have a test as tests/qunit/module.name/foo.test.js
- QUnit "modules" (not to be confused with ResourceLoader modules) should be named after the JavaScript module they are testing and prefixed with
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 guidelines for writing browser tests.