Manual talk:Coding conventions/JavaScript

Where MediaWiki devs agreed about this, I would like to be pointed to. It was inserted by Krinkle. Yoda is awesome. Yoda is readable. Save is Yoda, also. -- Rillke (talk) 14:22, 10 July 2012 (UTC)Reply

The Wikipedia article points out that the main cricism against this is cognitive load and counter-intuition when developing, reviewing, testing, and debugging code. These remain relevant today and why we don't use this style.

The points in favour of Yoda are, to my knowledge, limited to points about detecting mistakes in new code. They do not apply to how one would intentionally write code, or how any code is read or debugged later. That is where the majority of time is spent, and thus where the priority sits in my opinion.

In addition to that, the points about preventing mistakes in newly written code, refer either to unrelated program languages, or to issues that no longer apply to present-day compilers and linters, which all prevent, detect, or warn against such mistakes. Krinkle (talk) 18:02, 27 August 2021 (UTC)Reply

Please provide a script like jsBeautifier for automatically applying these white space conventions. Rillke (talk) 15:55, 17 July 2012 (UTC)Reply

+1 Helder 16:07, 17 July 2012 (UTC)

In the series of "ambiguous code styles that really shouldn't be ambiguous", what's the best form of loop in the situation of looping through an array?

I'm seeing all three of

for ( var i = 0; i < arr.length; i++ ) {
    ....
}
for ( var i in arr ) {
    ....
}
$.each( arr, function ( i, item ) {
    ....
} );

Input appreciated! MarkTraceur (talk) 17:55, 25 July 2012 (UTC)Reply

for (var i in arr) has to be armored with checks for arr.hasOwnProperty(i) to protect against brokenness if frameworks are present that modify the Array prototype. Bleh!

Personally I prefer to use $.each() most of the time:

• functional style / iterator feels nicer than counting manually
• you get a real function scope -- for instance you can safely use i in a lambda function inside your loop, which you can't with the first two forms

However it is a little more expensive, so performance-critical paths may prefer the traditional for loop with iterator variable and comparison to length. brion (talk) 18:01, 25 July 2012 (UTC)Reply

I myself prefer $.each(), mostly because of its functional nature. Besides, I love the jQuery way. Nischayn22 (talk) 07:00, 28 April 2013 (UTC)Reply

There is no one convention for looping in general, because there are different solutions for different situations.

Looping over an array

When it comes to looping over an array (or array-like objects such as jQuery objects and Argument objects) however, then there really is only one good way, and that's a for loop with a number counter. And the reason is that you want to loop over the the indexes, not over all properties unconditionally.

Example of dump for a jQuery object ($('#content');):

{
    selector: '#content',
    context: HTMLDocument.
    length: 1,
    0: HTMLDivElement
}
/// inherits from jQuery.prototype
/// inherits from Object.prototype

You wouldn't want to loop over all properties, only the indices (based on the length). Even a hasOwnProperty filter won't help you here, because they are all "own" properties.

The other case is when there is a sparse array that may contain gaps, in most cases the intent is to loop over all of them, and yield undefined for gaps. Another case is that in JavaScript everything is either a primitive (string, number, boolean, ..) or an object. Arrays are objects. And as such, keys of properties are strings, which can cause issues when accessed as such. A for-in loop will give you the internal key values, which are strings (as they should be).

e.g. the following will never log the "Success!" message:

var key,
    arr = ['foo', 'bar'];
for ( key in arr ) {
    console.log( 'Key:', $.toJSON( key ), 'Value:', $.toJSON( arr[key] ) );
    if ( key === 1 ) {
        console.log( 'Success! Found the second item (should be "bar"):' + arr[key] );
    }
}

Because arr (as being an object) is { "0": 'foo', "1": 'bar' }, and "1" !== 1.

Looping over an object

If you do want to loop over all properties of an object then, of course, there is nothing wrong with a for-in loop.

var obj = {
    'foo': 'Hello Foolifications'
    'bar': 'Rabarber Barbara\'s bar'
};
for ( key in arr ) {
    console.log( 'Key:', $.toJSON( key ), 'Value:', $.toJSON( arr[key] ) );
    if ( key === 1 ) {
        console.log( 'Success! Found the second item (should be "bar"):' + arr[key] );
    }
}

If you explicitly need to exclude inherited properties (e.g. you have an instance of FooBar that inherits from FooBar.prototype, and you want to clone it):

var clone, key,
    hasOwn = Object.prototype.hasOwnProperty,
    foo = new AwesomeFoo( /* .. */ );
/* .. */
clone = Object.create(AwesomeFoo.prototype);
for ( key in foo ) {
    if ( hasOwn.call( foo, key ) {
        clone[key] = foo[key];
    }
}

So, what about $.each ? Well, $.each is basically the same as a for-in loop ($.each does not filter hasOwn), with the difference that it has its own scope. In most cases you probably don't need it, but if you need to do a lot of loops and want to avoid conflicts with the different counter or key variables, then using a scope can be helpful. Especially in nested loops. Krinkle (talk) 23:46, 25 July 2012 (UTC)Reply

It should be mentioned that $.each is checking whether the given object/array has a length field. If so, then each will only iterate over the "numeric" fields smaller than "length", no matter whether the field's value is undefined or not. Danwe (talk) 19:09, 13 August 2013 (UTC)Reply

@MarkTraceur For a simple array loop I would use ES5's arr.forEach. The var i = 0 style is marginally faster for very large loops, and mostly just in older browsers (e.g. IE11).

$.each will currently trigger a linter error suggesting you use arr.forEach instead; there's no need to use jQuery when the plain JS versions is just as concise. ESanders (WMF) (talk) 16:36, 25 March 2021 (UTC)Reply

What is the convention for quotes - double or single?

It has little meaning, but would be nice for consistency.

jQuery suggests double. Amir E. Aharoni (talk) 17:44, 7 October 2012 (UTC)Reply

If I remember correctly it is single. Helder 11:15, 10 October 2012 (UTC)

Single quotes indeed.

For JavaScript files quotation is purely stylistic as string literals in JavaScript are specified as either wrapped in double quotes or single quotes. There is no magic functionality or different escaping (e.g. no need for double quotes to use \n or something).

In PHP we use both because of magic quotes, though in PHP we also prefer single quotes when magic functionality is not intended. Krinkle (talk) 00:54, 20 October 2012 (UTC)Reply

To my surprise, I just got a hint from a fellow developer that I am not doing whitespaces as stated by the conventions. e.g. I am writing if(...) {} rather than if (...) {}. This has been legal before Revision 547172 as of 13:45, 6 June 2012. That revision has introduced the line Keywords that are followed by ( should be separated by one space. I think the given justification (This helps distinguish between keywords and function invocations) is not really a good one. First, you should really know your JS keywords, second, function calls are not followed by any "{" but by ";", so this should make it clear as well.

Since the not so spacey convention is (for a long time already) explicitly allowed by the PHP coding conventions (which were the general conventions not so long ago), I would like to demand bringing these on the same level again, allowing the not so spacey style in JS as well. As the PHP conventions put it, "Opinions differ" on this, and yes, that's no different when it comes to JavaScript. Please let me know if there are any objections, otherwise I would just adjust this soonish.

In general, I would appreciate if coding conventions could be discussed a little more, or perhaps someone could just point me to where they are being discussed... Thanks. Danwe (talk) 15:18, 13 November 2012 (UTC)Reply

• Some operators require a space by syntax, others tolerate both (in the case of if, both are allowed). In that case it comes down to convention and consistency.
• Our JS code base is relatively new, therefore it doesn't make sense to start off with allowing arbitrary variations that are inconsistent in nature. Incorporating exceptions in the code conventions (the convention to separate all keywords by a space) might make sense for an established code base, but not for a new one.
• What do you mean by "know your keywords"? Are you suggesting someone might not know what is an operator and what a function? Though I think one should know what is and isn't an operator, that's easily looked up if you don't know, and then there is editors that do the highlighting for you, and of course the whitespace convention to make it easy to detect. And to be fair, it is a reasonable expectation to know the basic operators in a language before writing in it, of course (if, else, function, throw, ..).

As for discussion place, I think this talk page is as good as any.

Provided the following use cases for enforcing the "space after if":

• Consistency in style (sometimes a space and sometimes not is inconsistent)
• Consistency in syntax (some operators require it, some operators don't)
• Simplicity in distinguishing operators/keywords from function invocations.

Can you respond with a few use cases for leaving it "differed" ? Krinkle (talk) 01:58, 14 November 2012 (UTC)Reply

One could simply turn around your use cases if one would say that we are talking about spaces before parentheses rather than spaces after keywords, and perhaps that's actually the logical ting to do:

1. Consistency in style (sometimes a space before parentheses and sometimes not is inconsistent, e.g. someFunc() is very inconsistent with if (...) {...}).
2. Consistency in syntax (some operators require it, some operators don't) - Sorry, but I don't understand. I am not sure whether all keywords are operators, but not all operators are keywords. So we are not consistent here at all right now. E.g. ++i; rather than ++ i; or !someVar instead of ! someVar.
3. Simplicity in distinguishing operators/keywords from function invocations. So, that one is a contradiction to #1 now. But it's also not needed since you still have several indicators making clear whether the thing, preceding parentheses, is a keyword or a function-call:

• Your IDEs syntax highlighting (come on, probably all serious IDEs have this, even most modern debuggers do)
• Your knowledge (and yes, you understood me right when I said "know your keywords" :)
• Keywords followed by parantheses "( )" (control structures) will always be followed by braces "{ }" as well (per our own conventions where we don't allow Braceless control structures).
• Function calls should be followed by ";", control structures don't require this. The only exception here would be something like var foo = function() {};. This function example is actually also the only case where a keyword followed by parantheses, followed by braces, doesn't necessarily have subsequent, indented line.

I guess there could evolve a huge discussion around this and I am sure you are convinced that the current style is the one to go, I am convinced that this is the style which makes a lot more sense. For me the current style is just as inconsistent as the one I'd suggest is for you. There must have been a similar discussion when people decided over spacey or not so spacey style in PHP and perhaps we should learn from history and be consistent with PHP - it still is MW code and both languages have C-like syntax, so why not leave this rule to the common coding conventions. I believe the more rules are described on a common basis, the better. It makes things simpler and lets you concentrate on the essential things like actually writing code and getting stuff done rather than studying conventions for different syntactically very similar languages. Danwe (talk) 16:54, 22 November 2012 (UTC)Reply

The only argument I'm willing to push is consistency for exact usage and the reason thereof:

• Not if() in one module and if () in another.
• We don't need to tolerate both variations, because unlike the PHP code base, the JS code base is relatively new and we can easily enforce one variation and stick to it.

The other arguments such as consistency among different operators are harder to document because, as you say, ++ and ! are also operators. So let's not argue that now. If you want you could specify it as follows:
"A keyword followed by ( (left parenthesis) should be separated by a space."[1]

Which is effectively the same as what I meant with "Consistency in syntax (some operators require it, some operators don't)" because operators that don't require it are followed by a left parenthesis, and the ones that don't require a space by syntax.

Regarding "being consistent with PHP", we aren't inconsistent with PHP because our PHP code conventions don't enforce if(), it currently tolerates both. See my first point above.

Regarding "more common code conventions", I oppose this because of two reasons:

• It means we have to read two pages for a language's conventions instead of one.
• It encourages (and has led to) bad coding habits. Different languages have different syntaxes and internals, despite looking similar. Applying code structure conventions decoupled from the language is dangerous and harder to maintain and use. I've been working on phasing out those "common code conventions". For example, another PHP/JS project I work on preferred placing the curly place on the next line. This is fine in PHP where the distinction is purely stylistic, but in JS there are various cases where this changes the behaviour of the program (usually due to Automatic Semicolon Insertion).

I agree that "More rules" (that is, in order to removing ambiguity) is better. But more variations ("switch cases intended or not intended, if with or without space") or more edge cases ("foo like bar, except when bar is quuxified or when quuxification happens within baz") is not a good thing and serves no purpose what's however other than reducing consistency and asking for problems and conflict resolution. We stick with one and you get used to it.

Krinkle (talk) 01:39, 31 December 2012 (UTC)Reply

In many cases, if MW extensions bring some JavaScript, they create a module like object in the global scope which will hold all the extensions JS stuff, e.g. in further objects in fields of that extension object.

e.g. window.dataValues, window.dataValues.DataValue, etc.

I think there is common agreement that it is bad for extensions to pollute the global scope with those objects. Some have suggested putting those objects into the global mediaWiki object instead. This would result in mediaWiki.dataValues and mediaWiki.dataValues.DataValue etc.

I think this is equally bad. Take an extension "messages" for example. Putting it in mediaWiki.messages would overwrite the "messages" field introduced by MW core. You are simply moving the problem from global scope into the MW object and making it worse since the MW object is occupied by some stuff already. I think there should be some coding convention where to put those objects instead. In my opinion, a mw.ext would be the way to go. This would result in a mediaWiki.ext.dataValues and mediaWiki.ext.dataValues.DataValue. The slightly longer name doesn't really make it worse, and if you use it a lot in one file, you would still alias it via the files outer closure e.g.

( function( mw, dv, $, undefined ) {
	'use strict';
	// ... now you can use dv.DataValue here.
}( mediaWiki, mediaWiki.ext.dataValues, jQuery ) );

Per the conventions, no globals other than mw and jQuery should be used.

I agree adding third-party libraries on one of these globals directly is probably bad.

Extensions providing third-party libraries could be aliased under mw.libs if we don't want to use them directly.

As for the structure, I'd recommend this:

-- Provider
( function () {
    'use strict';
    /* local scope */
    /* public API */
    var dv = {
    };
    // Expose
    mw.dataValues = dv;
}() );
-- Consumer
( function () {
    'use strict';
    var dv = mw.dataValues;
}() );

Krinkle (talk) 19:53, 23 November 2012 (UTC)Reply

I agree, mw.libs should be separate. I guess we should offer a mw.ext then, so all extensions can start using that one rather than putting stuff in other places. I might upload a patch set soon, shouldn't be a big thing really. Danwe (talk) 20:56, 25 November 2012 (UTC)Reply

I noticed something in the mobile extension code and discussed it with Jon (although I'm not sure if I convinced him).

In the existing code a DOM element from a jQuery object is often passed instead of simply passing the jQuery object, e.g.:

setupReferences( $( '#content' )[ 0 ] );

Jon said that he wants to be sure that only one element is passed. I think it just makes the code look more confusing and doesn't really solve the problem. I mean we should simply assume that the selector selects only one element. If it selects more than one element it's obviously a mistake and we actually can't say if [0] is the right one or maybe [1] or [54]. If we really want to be sure we can check .length at the beginning of the invoked function.

Anyone disagrees? Could we include that somewhere in the coding conventions? JGonera (WMF) (talk) 19:17, 21 December 2012 (UTC)Reply

ID selectors are incapable of returning more than one element. So if you're referring specifically to code using [0] on an object from an ID selector, that doesn't deserve mention in code conventions as it is imho just plain stupid. Something you might be confused by once, but if we start including things like that there is no telling where it will end end. Code conventions isn't a training guide to how the browser works and how the DOM works.

For passing around 1 element, you can use .eq( 0 ) instead of [0] to yield a cloned jQuery object that only contains the first node of the jQuery object collection.

Regarding passing HTMLElement objects or jQuery collections between functions, it depends on what makes sense for the situation. I don't think this is the kind of thing where there is 1 right solution or where using different approaches in different modules makes code harder to use.

jQuery plugins obviously operate on a jQuery collection as they are methods on the jQuery prototype itself. Utilities often operate on 1 single element. In the latter case it is imho preferred to document the method as taking an HTMLElement as opposed to a "jQuery object that must have only 1 element". That way the responsibility of extracting the right data is in the hands of the caller. So that you have Foo.bar( $foo[0] ) instead of Foo.bar( $foo ). That way Foo.bar doesn't have to verify it has length, take the first one, and maybe throw an error if it has more than one?

Also note that constructing a jQuery object for a node is very cheap. It is the first executive use case in the constructor. Nothing to worry about there. Things like $( this ) in DOM callbacks are very common and very fast.

So something like this would be fine:

/** @param {HTMLElement} el */
Foo.getState = function (el) {
  return $(el).data('foo-state');
};

However if utilities are more efficient when they can cache data outside a loop, then you may want to change the function to take an array-like object instead of requiring the callee to make a loop where Foo.bar then has to do initialisation for every single call inside the loop.

So, instead of this:

/** @param {HTMLElement} el */
Foo.setStuff = function (el, stuff) {
  var state = Foo.getState(el) || {};
  state.stuff = Foo.normaliseStuff(stuff);
  return $(el).data('foo-state', state);
};
// One
Foo.setStuff($('#foo-container')[0], /* .. */);
// Many
$('.foo-items').each(function () {
    Foo.setStuff(this, /* .. */);
});

The following would likely be better (so that stuff is only normalised once):

/** @param {Array|jQuery} els */
Foo.setStuff = function (els, stuff) {
  var state, i, len;
  stuff = Foo.normaliseStuff(stuff);
  for (i = 0, len = els.length; i < len; i++) {
    state = Foo.getState(el) || {};
    state.stuff = stuff;
    $(els[i]).data('foo-state', state);
  }
};
// One
Foo.setStuff($('#foo-container'), /* .. */);
// Many
Foo.setStuff($('.foo-items'), /* .. */);

This sort of a bad example since jQuery methods support setting on the entire collection at once, so this is probably better done like $('.foo-items').data('foo-state-stuff, /* .. */); but you get the idea. Also, you could extend Foo.setStuff with els = els.nodeType ? [ els ] : els; so that it supports both:

/** @param {Array|jQuery|HTMLElement} els */
Foo.setStuff = function (els, stuff) {
  var state, i, len;
  els = els.nodeType ? [ els ] : els;
  stuff = Foo.normaliseStuff(stuff);
  for (i = 0, len = els.length; i < len; i++) {
    state = Foo.getState(el) || {};
    state.stuff = stuff;
    $(els[i]).data('foo-state', state);
  }
};
// Alternative:
/** @param {Array|jQuery|HTMLElement} els */
Foo.setStuff = function (els, stuff) {
  $(els).data('foo-state-stuff', stuff);
};
// Alternative (2):
/** @param {jQuery} $els */
Foo.setStuff = function ($els, stuff) {
  $els.data('foo-state-stuff', stuff);
};
// One (node)
Foo.setStuff($('#foo-container')[0], /* .. */);
Foo.setStuff(document.getElementById('foo-container'), /* .. */);
// One
Foo.setStuff($('#foo-container'), /* .. */);
// Many
Foo.setStuff($('.foo-items'), /* .. */);

Krinkle (talk) 01:09, 31 December 2012 (UTC)Reply

@Krinkle

(I suggest that) perhaps a shortened version of your reply should go to the article page. Wikinaut (talk) 08:10, 31 December 2012 (UTC)Reply

Thanks for the explanation. Maybe I sounded stupid bringing up a problem like that. It just bothered me to see $(something)[0] all around the code and I needed some arguments to convince Jon to stop doing that :) JGonera (WMF) (talk) 00:13, 4 January 2013 (UTC)Reply

I have been thinking and researching about the topic of multiple vs. single var statements recently and have changed my ways entirely. I switched from using var do declare bunches of variables at the same time to using one var per line. Here are some reasons, going so far that I would completely discourage from using a single var for anything more complex than var a, b, c;

• Multiple var are better readable in diffs. Each line ends with a ";" and is completely self-contained. E.g.:

var whatever = 0,     whatever2 = 2;	vs.	var whatever = 0;  var whatever2 = 2;
Two lines changed		One line changed

The multiple var version will therefore keep the git blame for the first line in tact, which is a huge advantage. Changing a line of code as a side effect of syntactical sugar or preferences is bad.

• Huge difference when debugging. If you have a list of five variables using one var and you want to see the value of the second one before creating the third one, then this is not really possible in any debugger known to me since all five variables are created in one step.
• Whenever tabs are displayed with 8 spaces, the whole one var thing looks incredibly ugly:

var foo = 123, bar = 321;

vs.

var foo = 123; var bar = 321;

• When assigning initial values does not fit into one line, one is left with the question how to spread the assignment over multiple lines and I have seen various models, none of which makes me feel comfortable when looking at. Some examples (there are many more variations, you get the idea I hope):

// Quick skimming over the code could give you // the expression "xxx" is a variable as well. var foo = foo( xxx + "someverylongstring..." ), anothervar = false;

// less misleading but still kind of ugly var foo = foo( xxx + "someverylongstring..." ), anothervar = false;

var foo = { field: 1 }, bar = { field: 2 };

vs.

var foo = { field: 1 }; var bar = { field: 2 };

or, adding tabs for formatting the first var as soon as the second gets added, sucks a lot when diffing again:

var foo = { ■■■■    field: 1 ■■■■};,     bar = {         field: 2     };

vs.

var foo = {     field: 1 }; var bar = {     field: 2 };

• Multiple var offer higher aesthetics for comments on top of variables without even thinking about it:

// some comment about foo var foo = true, // some come comment about bar bar = false; or (better, but haven't seen this one so often) // some comment about foo var foo = true, // some come comment about bar bar = false;

vs.

// some comment about foo var foo = true; // some come comment about bar var bar = false; Compared to the first multi var version this gives more space for comments and does not look odd or like the one comment is more important than the other one. Also, the first single var version would look quite horrible in a diff when removing the first variable declaration.

Most of this has also been covered in a more extensive article by Ben Alman.

I am not saying we should change all var statements now, but add this to the coding guidelines and have an eye for it during code reviews. Currently the convention does even discourage from using multiple var, I highly disagree with this. Danwe (talk) 18:43, 13 August 2013 (UTC)Reply

Recently I have been wondering how to document whether a object (e.g. required by a function's parameter) had to be an instance of a certain constructor vs. more basic requirement that it had to match a certain interface definition.

An example for this is jQuery.Promise. jQuery.Promise is no constructor but certainly is some sort of interface definition that you could do duck-type checks against.

Rather than just documenting that a parameter requires {Object}, we usually write {jQuery.Promise}. It is not clear though whether an instance of that constructor (in this case there is not even a constructor) is required or whether the object only had to stand duck-type checks against the interface. Is there any usual way how to add this information to the documentation? I could immagine something like jQuery.Promise^, so the ^ would imply that no real instance is required. Danwe (talk) 15:35, 28 August 2013 (UTC)Reply

I would document it as {jQuery.Promise}. In fact, I have done so in a couple cases already. It is a documented type, and there's specific code implementing it. Based on the implementation and the target option of deferred.promise, I would describe jQuery.Promise as a [:en:mixin mixin], rather than an interface.

Despite there being no Promise constructor, it's relatively easy to make a promise; for example, the caller can call the jQuery.Deferred constructor, then call deferred.promise. Technically, the second step is optional, as all Deferred objects are also Promise objects. However, calling .promise is necessary if you want to prevent external code from resolving your Promise.

Although duck-typing may work for Promise sometimes, I don't really see the need to advertise that in this case. Superm401 - Talk 23:14, 1 September 2013 (UTC)Reply

Sorry, but this is not really what I wanted to know. Promise was just an example for something that can only be checked against by duck-typing and in documentation as some sort of "pseudo constructor" or concept, basically an interface definition.

You can't describe jQuery.Promise as a mixin since there is no jQuery.Promise. {jQuery.Promise} is just that conceptual thing we refer to documentation as described above. Besides, mixins are basically interfaces with implementation. Or you could argue behind each mixin there should be an interface conceptually.

So the question remains, how to document that something is rather referring to some concept rather than a real physical thing (a constructor). Danwe (talk) 19:37, 19 September 2013 (UTC)Reply

While jQuery.Promise may (implementation-wise) not be a public constructor, I would most certainly consider it a class. In essence it is a private class in jQuery that various other classes inherit from by calling the private object constructor function for promises. And while that constructor is not exposed, the objects constructed from it are.

jQuery.Deferred objects mixin all public properties of the internally constructed promise. And deferred.promise() returns the promise object as-is. JavaScript doesn't really have classes to begin with anyway, and as such it is up to a constructor to decide whether to construct objects plainly (inheriting only from Object), or with additional prototypes in the chain. In the case of jQuery's promises, they are plain objects.

Anyway, in jsduck documentation nothing implies that it has to pass instanceof in execution context. It should however pass "instance of" conceptually (an object created by the class documented in jsduck under that name). The idea of "^" doesn't make sense in that case because for all intends and purposes the object is and must in fact be an instance of jQuery.Promise.

If the function you're documenting is invoked with a promise created by jQuery.Deferred (or its callers such as $.ready, jqXHR, jQuery.fn.promise etc.) then using jQuery.Promise seems most appropriate within the conventions of how we use jsduck.

If the function takes a compatible promise created by another library, then I agree we could have an abstract class definition for Promise that could be used for cases where you aren't dealing with promises made by jQuery per se. Be careful though with assuming what is part of a "standard" promises. The specification is still in flux. It's easy to assume you need a simple promise and end up relying on jQuery-specific behaviour. Krinkle (talk) 15:10, 21 April 2014 (UTC)Reply

The convention to put spaces around function parameter lists was not documented. It is documented for PHP, and is de facto for JS. However, I think it's better to add it specifically here. Superm401 - Talk 21:54, 21 January 2014 (UTC)Reply

Done by you in these two edits. +1. :-) Jdforrester (WMF) (talk) 17:30, 22 January 2014 (UTC)Reply

Hi,

The page says "We use JSHint as our code quality tool."

But when I use JSHint it tells me not to use space between parenthesis and parameters. So this is not consistent.

Thank you to enlighten me. Automatik (talk) 00:31, 6 April 2014 (UTC)Reply

JSHint is a code quality tool, not coding style. It has some legacy options for whitespace enforcement, however those are deprecated, disabled by default, and our configurations don't enable that option (as reflected by the lint job that runs from Jenkins, which uses the same configuration, and as such should also never enforce or warn for options like that).

The whitespace style option in JSHint is not flexible (it was a boolean option for backwards-compatibility with JSLint to enforce Crockford's code conventions).

If JSHint is warning you about those whitespace rules, make sure you find out why that option is enabled and disable it. Krinkle (talk) 14:52, 21 April 2014 (UTC)Reply

Actually I was using JSLint instead of JSHint which does not consider these spaces as an error.

Thanks for the expanations furthermore. Automatik (talk) 16:57, 25 April 2014 (UTC)Reply

Under "Line length", there is an example mw.foo.getThatFrom('this'). Should it be mw.foo.getThatFrom( 'this' )? Danmichaelo (talk) 12:50, 24 July 2014 (UTC)Reply

Indeed; I've fixed this. Thanks for the spot. Jdforrester (WMF) (talk) 16:25, 24 July 2014 (UTC)Reply

What is the coding convention for whitespace inside of square brackets?

var array = ['foo', 'bar'];

or

var array = [ 'foo', 'bar' ]; 

Fomafix (talk) 23:42, 7 February 2014 (UTC)Reply

None should be used adjacent to the brackets (i.e. the first is correct). The only exception I can think of is when there's a line break immediately after the first bracket (generally used for a long list, or at least a long item). Superm401 - Talk 03:06, 8 February 2014 (UTC)Reply

I think this is outdated – I believe that the rule is now you always have spaces when creating an array, but never when accessing:

Right

 var array = [ 'foo', 'bar' ];

 array[0].print();

Wrong

 var array = ['foo', 'bar'];

 array[ 0 ].print();

Jdforrester (WMF) (talk) 23:19, 10 February 2014 (UTC)Reply

+1 - This seems much more consistent with the space rules for normal brackets ( and ). Danwe (talk) 08:10, 18 February 2014 (UTC)Reply

I doubt whether we need to avoid space while accessing it. When in doubt I usually refer https://contribute.jquery.org/style-guide/js/#arrays-and-function-calls since that is the closest convention to MediaWiki(double quotes vs single quote being major difference). According to that, conventions for white spaces in arrays and function calls

Right

array = [ "*" ];
array = [ a, b ]; 
foo( arg ); 
foo( "string", object ); 
foo( options, object[ property ] ); 
foo( node, "property", 2 );

This also makes it consistent with rules for normal brackets-(). "When a function has parameters, there should be a space on the inside side of both parentheses, for both function definitions and function calls."

I think we need to decide on this and update the Manual. I see square brackets used with and without white space in our codebase(example). Thanks. Santhosh.thottingal (talk) 11:58, 31 March 2014 (UTC)Reply

I agree with Santhosh - consistency with all kinds of brackets and with the jQuery conventions is a good thing.

Just to be sure - this also includes constructs such as array[ i ], right? So

• array[ i ] - correct
• array[i] - incorrect Amir E. Aharoni (talk) 09:02, 1 April 2014 (UTC)Reply

Would be consistent. Also, array[ 0 ]. Danwe (talk) 02:30, 10 April 2014 (UTC)Reply

Now done in this diff. Jdforrester (WMF) (talk) 16:26, 17 January 2015 (UTC)Reply

I'll note that what WMF has for a coding convention is exactly the opposite to what JSlint thinks it should be. It's a real pain to have to JSlint my code to meet other stuff and then have to go back and manually add the MWF CC reguarding whitespace. I propose that either an alternative to JSlint be made available for WMF coding conventions or that the coding conventions be made to comply with the JSlint the rest of the world uses. Thanks for reading and I look forward to responses. Technical 13 (talk) 18:27, 18 January 2015 (UTC)Reply

Use jscs with the "wikimedia" preset and you'll be great. Jdforrester (WMF) (talk) 01:02, 19 January 2015 (UTC)Reply

What's jscs? Is it a plugin for Notepadd++? Where do I DL it or find documentation on it? Technical 13 (talk) 03:12, 19 January 2015 (UTC)Reply

jscs == JavaScript Code Style checking module: http://jscs.info/

We use it at Wikimedia to automatically spot violations of the coding conventions. Each repo has it's own configuration, but over time we're slowly converging each repo to compliance with the complete set of the coding conventions that we have.

I've never heard of "Notepad++" but I assume it's a text editor you choose to use. I don't know if it has a jscs plugin; the editor I use (Atom) does, which is very useful. Jdforrester (WMF) (talk) 16:17, 20 January 2015 (UTC)Reply

Is there a plugin for Notepad++ for JShint for these coding conventions? The default one doesn't seem to work very well. It uses an entirely different spacing convention and doesn't have all the mw. globals predefined. Technical 13 (talk) 22:21, 13 January 2015 (UTC)Reply

I understand we should not manipulate the prototype (and I'm fine with the conventions saying that).

However, @Sbisson (WMF) noted that user scripts sometimes manipulate the prototype anyway. So it seems we should take defensive measures when looping nonetheless. Mattflaschen-WMF (talk) 00:38, 23 November 2016 (UTC)Reply

Any such scripts should be strongly advised to stop doing that, and definitely not made available as gadgets. I don't think we need to consider how we support them though - that would be like asking "how do we protect against as user script that has window.mw = null;" ESanders (WMF) (talk) 12:58, 4 December 2016 (UTC)Reply

mw:Manual:Coding conventions/JavaScript#Whitespace: "Indentation with tabs."

Not so in the examples.

No time to fix, sorry. Jack who built the house (talk) 04:15, 12 October 2017 (UTC)Reply

Done → User: Perhelion 08:50, 12 October 2017 (UTC)Reply

I've always wondered why tabs whereas the wikicode editor doesn't provide it with the tab key. JackPotte (talk) 07:46, 12 October 2017 (UTC)Reply

CodeEditor (which opens when editing pages with "javascript", "css" and "scribunto" content models) does. Jack who built the house (talk) 07:50, 12 October 2017 (UTC)Reply

Is there any reason why `Object.keys( x ).length` (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/keys) isn't recommended for getting size of objects (in section "Collections")? Having a loop for this seems odd to me. Sebastian Berlin (WMSE) (talk) 11:40, 20 October 2017 (UTC)Reply

The only reason seems https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/keys#Browser_compatibility. As this old browsers needs no support or recommendation anymore, you can freely add this example. → User: Perhelion 13:42, 20 October 2017 (UTC)Reply

The example was added in https://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/JavaScript&diff=2589439. Fomafix (talk) 10:45, 18 January 2018 (UTC)Reply

Check eslint-config-wikimedia #360 on GitHub. Krinkle (talk) 00:48, 18 March 2021 (UTC)Reply

This landed last year; documentation updated. Jdforrester (WMF) (talk) 18:01, 23 February 2022 (UTC)Reply

RESOLVED

Done with wikimedia/eslint-config-wikimedia 0.26.0

With the advent of the frontend stable interface policy, there's a need to mark things as stable. This is especially true in the near-term as most frontend code doesn't have any indication of stability, so visibly seeing @stable tells the developer that method/class is there to stay.

JSDoc/JSDuck does not currently support @stable but a similar feature has been proposed. While our generated docs won't do anything special when they see this tag, it'd be nice to be able to use it without eslint yelling.

Thus, I'm proposing we allowlist @stable through the use of the definedTags option for the check-tag-names rule.

Later, once the new frontend stable interface policy has matured, we may wish to be more granular like we do for PHP and tag things as "@stable to call", "@stable to extend", etc., but for now just being able to use "@stable" at all (without having to add an ignore rule) I think is a good start. — MusikAnimal ^talk 21:26, 17 October 2023 (UTC)Reply

Support Sounds like a good idea to me. Sam Wilson 05:40, 18 October 2023 (UTC)Reply

Support Jdlrobson (talk) 18:39, 20 October 2023 (UTC)Reply

Hi all, I'd like to discuss documentation comment conventions for classes as part of the migration to JSDoc. In JSDoc, specifically for classes:

• The main description in the documentation comment (or the @description tag) is interpreted as the description of the constructor
• The @classdesc tag is interpreted as the description of the class^[1]

These two description fields appear in the documentation site as:

# mw.MyClass
Class description

## Constructor
### new mw.MyClass()
Constructor description

There are two ways this can be formatted in the comment:

1. Constructor description first: Simpler but in reverse order from how the information appears on the documentation site

/**
 * Constructor description first line.
 * 
 * Constructor description continued...
 * 
 * @classdesc Class description first line.
 * 
 * Class description continued...
 * 
 * @param string myParam
 */

2. Class description first: More verbose but reflects the same order as the documentation site

/**
 * @classdesc Class description first line.
 * 
 * Class description continued...
 * 
 * @description Constructor description first line.
 * 
 * Constructor description continued...
 * 
 * @param string myParam
 */

The empty lines aren't required by JSDoc, but I've included them for readability and consistency with Manual:Coding conventions/Documentation.

Which option should we standardize on? Ideally, we'd be able to use @constructor instead of @description, but JSDoc doesn't support that. Of the two options, I think option 2 is the least ambiguous and easiest to reason about.

I think developers have a tendency to, whatever comes first in that block, is where we put high-level explainers, detailed descriptions, and usage examples for the class as a whole.

Having that be tucked away in the constructor description would be unfortunate and make the information less discoverable and thus waste investments in it somewhat.

For that reason, and for parity with PHP conventions wherever possible, I'd suggest option 2.

@APaskulin (WMF) I believe your code example is for the lower-level ("ES5-style") class, where the class and constructor are defined together as function FooBar(…) {…}.

When ES6 syntax sugar is used (which makes it look like the class and constructor are separate things), there is usually two separate doc blocks:

class FooBar {
 constructor() {…}
}

Can you add an example for this as well? Does it require the same tags? I'm hoping @description would no longer be needed in that case, and would @class stay in the first block? Krinkle (talk) 19:39, 4 March 2024 (UTC)Reply

That's a great point. This is much cleaner and works correctly:

/**
 * Class description.
 */
class myClass {
	/**
	 * Constructor description.
	 * 
	 * @param ...
	 */
	constructor() {...}
};

Adding a @class tag to the ES6 example (in either comment) actually messes it up and causes JSDoc to duplicate the class.

In the ES5 example, the @class tag isn't necessary in most cases, but it doesn't break anything. I included it because we have a lot of @class tags in MediaWiki core, but I'll update that post to remove it since it's not required. APaskulin (WMF) (talk) 20:18, 4 March 2024 (UTC)Reply