2509
Comment:
|
3642
|
Deletions are marked like this. | Additions are marked like this. |
Line 5: | Line 5: |
These are items that any [[ReviewerSchedule|code reviewer]] should keep in mind when reviewing JavaScript. |
|
Line 7: | Line 9: |
* '''JAVASCRIPT FILES MUST BE LINT-FREE''' * '''NO NEW SCRIPT FILES SHOULD BE PULLED IN BY THE YAHOO COMBO-LOADER''' |
* '''ALL JAVASCRIPT FILES MUST BE LINT-FREE''' * '''NO NEW SCRIPT FILES SHOULD BE PULLED IN BY THE YAHOO COMBO-LOADER''' (use [[JavaScriptBuildSystem|these instructions]] to add the scripts properly) |
Line 14: | Line 16: |
* If 3 or more lines of strings are to be joined together, use the [].join("") form. | * If 3 or more lines of strings are to be joined together, use the `[str1, str2, ...].join("")` form. |
Line 17: | Line 19: |
* All functions, methods, and object properties should be described with JSDoc comment strings. | * All functions, methods, and object properties should be described with [[http://developer.yahoo.com/yui/yuidoc|JSDoc comment strings]] and [[http://developer.yahoo.com/yui/yuidoc#tags|tags]]. |
Line 20: | Line 22: |
* Function, method, and variable names should follow the [[http://www.python.org/dev/peps/pep-0008/|PEP 0008]] style guide (and no, this is not how the rest of the world does it). == XHTML, CSS, and Styling == |
|
Line 21: | Line 28: |
* CSS `url()` directives should have the file name surrounded by single-quotes: `url('spinner.png')` * XHTML attributes should be surrounded by double-quotes: `<input type="text"/>` * Setting a node's style explicitly with `setStyle()` should be avoided. * To set a node's style, it is preferable to add a CSS class to the node, and let the stylesheet handle the presentation. |
|
Line 25: | Line 36: |
* "`e`" should be used as the event object variable name in event handlers: `function onclick(e) {...}` | |
Line 40: | Line 52: |
* Modules should declare a `PROJECTNAME` namespace. | * Modules should declare a `PROJECTNAME` namespace: `Y.namespace("foo");` |
Line 43: | Line 55: |
* It is recommended to have a `@namespace` tag below the `@module` tag. | |
Line 48: | Line 61: |
* All protected methods should have the `@protected` JSDoc tag, even ones like `initializer` and `renderUI`. | * All protected methods should have the `@protected` JSDoc tag, even ones like `initializer()` and `renderUI()`. |
Line 50: | Line 63: |
* Custom events should be documented immediately preceding the line on which they are published. | * Custom events should be documented on the line immediately preceding the line where they are published. |
Line 53: | Line 66: |
* Widget should have a functional example page. | * All Widgets should have a functional example page. |
JavaScript Review Notes
These are items that any code reviewer should keep in mind when reviewing JavaScript.
CRITICAL
ALL JAVASCRIPT FILES MUST BE LINT-FREE
NO NEW SCRIPT FILES SHOULD BE PULLED IN BY THE YAHOO COMBO-LOADER (use these instructions to add the scripts properly)
Plain Old JavaScript
JavaScript blocks should be indented by 4 spaces.
If 3 or more lines of strings are to be joined together, use the [str1, str2, ...].join("") form.
- 1 and 2 character variable names are to be avoided.
- Regular expression literals should be avoided.
All functions, methods, and object properties should be described with JSDoc comment strings and tags.
C++ style comments ("//") should only be used inside function bodies.
Per-line var statements are preferred inside function bodies.
Function, method, and variable names should follow the PEP 0008 style guide (and no, this is not how the rest of the world does it).
XHTML, CSS, and Styling
CSS files should come before JavaScript files in the document head.
CSS url() directives should have the file name surrounded by single-quotes: url('spinner.png')
XHTML attributes should be surrounded by double-quotes: <input type="text"/>
Setting a node's style explicitly with setStyle() should be avoided.
- To set a node's style, it is preferable to add a CSS class to the node, and let the stylesheet handle the presentation.
Event Handling
"e" should be used as the event object variable name in event handlers: function onclick(e) {...}
event.halt() should be preferred over event.stopPropagation().
Document-wide keypress handlers are dangerous, and should be detached as soon as they are no longer needed.
JavaScript Modules
Modules should have a JSDoc string with the @module or @submodule tag.
Module names should have the form PROJECTNAME.foo.
- Module-body content does not have to be indented.
- Module-wide constants should go at the top of the file, after the module JSDoc string.
Multi-line var statements may be used in the module-wide constants block.
Namespaces
Modules should declare a PROJECTNAME namespace: Y.namespace("foo");
A module's public functions should be attached to the PROJECTNAME namespace object.
A module's public functions should not be attached directly to the Y object.
It is recommended to have a @namespace tag below the @module tag.
Widgets and Plugins
- Protected and private method names should be prefixed with an underscore.
All protected methods should have the @protected JSDoc tag, even ones like initializer() and renderUI().
Event handler method names should be prefixed with _on or _after.
- Custom events should be documented on the line immediately preceding the line where they are published.
All JSDoc strings for attributes should have @attribute, @type, and @default tags.
Objects such as Node instances should not be instantiated in the widget's ATTRS hash using value:, use valueFn: instead.
- All Widgets should have a functional example page.
(Not implemented) Widget example pages should follow the Example page Template.
- The widget unit test suite should leave a clean DOM tree in between tests.