jQuery Grid Plugin - jqGrid

Hello Tonny,

because jqGrid is the main and most important control in my last project I decide to spend more time in the analyze of JSLint suggestions to jqGrid code.

Now I place without any comments different suggestions unsorted. Important real bug fixes are mixed below with less important performance improvements and style changes. I bag pardon of all who read my bad English below.

But, before all, one general remark: JSLint suggest to use ‘!==’ instead of ‘!=’ and ‘===’ instead of ‘!=’ to suppress possible type conversion during compare values and to have strange effects with undefined variables. Another general suggestion is the usage of {...} block in all ‘if’ statements. It seems to me, that you hold the role currently, but in some most old places there is no {...} block. What do you think about inserting {...} block everywhere in the code.

----------------------------------------

The line 267 of grid.celledit.js:

window.setTimeout(function() { info_dialog($.jgrid.errors.errcap, v + " " + cv[1], $.jgrid.edit.bClose) }, 100);

hat missing semicolon and should be replaced to

window.setTimeout(function() { info_dialog($.jgrid.errors.errcap, v + " " + cv[1], $.jgrid.edit.bClose); }, 100);

The line 290

if ($.isFunction($.fn['datepicker'])) {

Should be better rewritten as

if ($.isFunction($.fn.datepicker)) {

Line 420 and 442 has unnecessary semicolon at the end of line.

Line 474

res["id"] = this.id;

can be better rewritten as

res.id = this.id;

-------------------------------------------------------

File grid.common.js

Function DaysArray define in the line 591 and used in the line 557 should be better renamed to daysArray to hold the name conversion.

Line 602

if (val.match(/^s+$/) || val === "") {

of the function isEmpty should better fixed to

if (val !== null || val.match(/^s+$/) || val === "") {

Function isArray from the lines 244-250 are used only in the line 1308 of grid.formedit.js. In grid.formedit.js isArray should be replaced to jQuery.isArray like in all other places of jqGrid and the function isArray removed from the lines 244-250 of grid.common.js. If you decide to hold isArray in grid.common.js, it should be better fixed in the line 245 from

if (obj.constructor.toString().indexOf("Array") == -1) {

To

if (obj && obj.constructor.toString().indexOf("Array") == -1) {

-------------------------------------------------------

The lines 79-80 of the file grid.custom.js

$(newtable).attr({id:defgrid['id']});
newtable.className = defgrid['cl'];

could be rewritten to

$(newtable).attr({id:defgrid.id});
newtable.className = defgrid.cl;

In the line 95

$t = this;

Would be better to insert “var”:

var $t = this;

In the line 106

if($t.p.footerrow) {$(".ui-jqgrid-sdiv","#gbox_"+$s.p.id).slideUp("fast");}

there is a bug with using undefined variable $s. It should be fixed to $t.

The line 112

if(ts.p.toppager) {$(ts.p.toppager).slideDown("fast");}

Should be changed to

if($t.p.toppager) {$($t.p.toppager).slideDown("fast");}

switch from the line 263

switch (this.stype) {

    case 'select' :

       ...

    default:

       ...

 

can be better rewritten to

if (this.stype === 'select') {
    ...
} else {
    ...
}

In the line 422: the body of a ‘for’ in should be wrapped in an ‘if’ statement to filter unwanted properties from the prototype. All body or for could be placed inside of body of following ‘if’ statement:

if (oSv.hasOwnProperty(key)) {
    ...
}

The same problem exists in the ‘for’ loop in the line 740.

Variable ov in the line 424 and later are used out of scope of declaration. So it would be better to move declaration of ov variable from the line 413 to the line 409 or 410. (To be exact, it is not a real bug in JavaScript code. It is known, that a variable declared in a block is visible everywhere in the function containing the block. But to be more close to classical computer languages and to make the code more readable it would be much better to hold role of scope visibility nevertheless.)

The same problem exists in the line 741. Declaration of ov variable could be moved from the line 732 to the line 728.

The variable td1 defined in the line 377 are not used.

----------------------------------------------------------------------------

In the file grid.formedit.js there is also some problems.

First of all, the line 102-105 looks like a little strange. To fix the problem one should remove declaration of variable searchable from the line 100 and insert “var ” at the beginning of the line 102. Then lines

searchable = (typeof v.search === 'undefined') ?  true: v.search ,

hidden = (v.hidden === true),

soptions = $.extend({}, {text: colNames[i], itemval: v.index || v.name}, this.searchoptions),

ignoreHiding = (soptions.searchhidden === true);

Will be look like

var searchable = (typeof v.search === 'undefined') ?  true: v.search ,

    hidden = (v.hidden === true),

    soptions = $.extend({}, {text: colNames[i], itemval: v.index || v.name}, this.searchoptions),

    ignoreHiding = (soptions.searchhidden === true);

A close problem exist in lines 681-684

var opt = $.extend({}, this.editoptions || {} ,{id:nm,name:nm});
frmopt = $.extend({}, {elmprefix:'',elmsuffix:'',rowabove:false,rowcontent:''}, this.formoptions || {}),
rp = parseInt(frmopt.rowpos) || cnt+1,
cp = parseInt((parseInt(frmopt.colpos) || 1)*2);

Which can be fixed with replacing ‘;’ to ‘,’ at the end of line 681:

var opt = $.extend({}, this.editoptions || {} ,{id:nm,name:nm}),
    frmopt = $.extend({}, {elmprefix:'',elmsuffix:'',rowabove:false,rowcontent:''}, this.formoptions || {}),
    rp = parseInt(frmopt.rowpos) || cnt+1,
    cp = parseInt((parseInt(frmopt.colpos) || 1)*2);

Lines 1467-1468

vwidth = window.innerWidth,
vheight = window.innerHeight

Should be replaced to

vwidth = window.innerWidth;
vheight = window.innerHeight;

The same problem in the lines 1470-1471

vwidth = document.documentElement.clientWidth,
vheight = document.documentElement.clientHeight

could be fixed to

vwidth = document.documentElement.clientWidth;
vheight = document.documentElement.clientHeight;

Line 42 has following error: “Function statements cannot be placed in blocks. Use a function expression or move the statement to the top of the outer function.” Function applyDefaultFilters declared in the lines 42-75 are used only in 170. Either one should move declaration of function applyDefaultFilters in the other place or insert body of this function in the line 170.

Lines 53, 57, 58, 60, 62, 63, 65 should be better rewritten in dot notation. For example

filterSettings['sFilter']

Should be replaced to

filterSettings.sFilter

In the lines 63, 69, 792, 1147, 1328, 1399 and 1436 have missing semicolon.

The scope of visibility of the variable fid defined in the line 77 ended in the line 185 und it’s usage in the lines 199, 212, 225, 226,228 and 230 are illegal. To fix this problem, the line 77 could be moved for example in the line after line 39.

Lines 80, 172, 173, 297, 544, 561, 711, 1123, 1140, 1144, 1232, 1240, 1267 and 1420 have unnecessary semicolon.

The ‘for’ loop from the line 136 should be modified so, that it’s body be wrapped in an if statement to filter unwanted properties from the prototype. I mean to insert an additional if like

if (eov.hasOwnProperty(key)) {
    ...
}

The same problem exists in the loop from the line 851.

----------------------------------------------------------------------

In the file grid.import.js.

Declaration of variable jstr in the line 35 should be removed, because it is already defined in the previous line.

Variable jstr1 used out of scope in lines 42, 44, 46. To fix the problem one can move declaration of from the line 36 in the line 34.

The body of a ‘for’ loop from the line 36 should be wrapped in an ‘if’ statement to filter unwanted properties from the prototype.

After all these changes lines 34-36

var jstr = xmlJsonClass.xml2json(cnfg," ");
var jstr = $.jgrid.parse(jstr);
for(var key in jstr) { var jstr1=jstr[key];}

Will be looks so:

var jstr1, jstr = xmlJsonClass.xml2json(cnfg, " ");
jstr = $.jgrid.parse(jstr);
for (var key in jstr) { if (jstr.hasOwnProperty(key)) { jstr1 = jstr[key]; }}

The body of ‘for’ loop from the line 153 has the same problem and the loop body could be placed inside of the ‘if’ body

if (gprm.treeReader.hasOwnProperty(key)) {
    ...
}

The line 166

ret=ret.replace(/}]}"/,'}]}');

contained unescaped character ']' and two unescaped characters '}'.

The line 182

$t = this;

Should be fixed to

var $t = this;

------------------------------------------------------------------

File grid.inlinedit.js:

In the line 54 svr['id'] should be better rewritten as svr.id. In the line 226 $.fn['datepicker'] should be rewritten as $.fn.datepicker.

Line 196 contains unnecessary semicolon.

-----------------------------------------------------------------

File grid.jqueryui.js:

There are missing semicolon in the lines 15, 31, 42, 74, 87, 100, 120, 174, 175, 291, 303 and 448 and unnecessary semicolon in the lines 51 and 450.

The body of a ‘for’ from the line 384 should be wrapped in an ‘if’ statement to filter unwanted properties from the prototype.

The line 476

var optstest = "{'\#gview_"+$t.p.id+" .ui-jqgrid-bdiv\':true,'" +opts._alsoResize_+"':true}";

contains bad escapement.

Lines 262, 314, 456 should be better rewritten in dot notation.

Lines 201-204

if (opts.classname) {
    selector.addClass(classname);
    select.addClass(classname);
}

Should be fixed to

if (opts.classname) {
    selector.addClass(opts.classname);
    select.addClass(opts.classname);
}

In the line 477 it would be probably better replace eval function to safer version.

--------------------------------------------------------------------

File grid.loader.js:

In the lines 45 and 46 unnecessary semicolon should be removed.

-------------------------------------------------------------------

File grid.setcolumns.js:

In the lines 54 and 76 one should replace

for(i=0;...

with

for(var i=0;...

The line 60 has missing semicolon.

-------------------------------------------------------------------

File grid.subgrid.js:

In the line 19 one should replace

for(i=0;...

with

for(var i=0;...

The line 82

dp[ts.p.prmNames['subgridid']]=sid;

should be better rewritten to

dp[ts.p.prmNames.subgridid]=sid;

The line 70 contain unnecessary semicolon.

-------------------------------------------------------------------

File grid.tbltogrid.js:

The line 101 contains definition of 'a' which is already defined in the line 84. It would be better to move declaration of 'a' from the line 84 to a line before 'for' loop (before line 84).

The line 105 contain unnecessary semicolon.

-------------------------------------------------------------------

File grid.treegrid.js:

The line 277

len = result.length;

Should be replaced with the line

var len = result.length;

The body of a ‘for’ from the line 123 should be wrapped in an ‘if’ statement to filter unwanted properties from the prototype.

-------------------------------------------------------------------

File jquery.fmatter.js:

The line 100

dateFormat=["i18n"];

declare and initialize variable dateFormat as an array object. The only place where dateFormat used as an array in the next statement (lines 102-105):

dateFormat["i18n"] = {
    dayNames:   opts.dayNames,
    monthNames: opts.monthNames
};

Later in the code only dateFormat.i18n is used. So it would be a little effective replace array with the only element dateFormat["i18n"] to an object like dateFormat_i18n. Then lines 100-105

dateFormat = ["i18n"];

// Internationalization strings

dateFormat["i18n"] = {

    dayNames:   opts.dayNames,

    monthNames: opts.monthNames

};

can be replaced to

dateFormat = {

    dayNames:   opts.dayNames,

    monthNames: opts.monthNames

};

And later in the code all dateFormat.i18n should be replaced to dateFormat.

In the line 215 ',' should be replaced to ';'.

The line 236 should be started with 'var '.

Lines 296, 336, 361, 365, 368, 370 and 429 contains missing ';'.

Lines 90, 91, 107 and 108 contains unescaped '-'.

Lines 470 and 474 contain unnecessary semicolon.

-------------------------------------------------------------------

File jquery.fmatter.js:

The line 331: 'custom_data' is already defined.

The line 507

o = $.trim(o).replace(/\&nbsp\;/ig,'').replace(/\&#160\;/ig,'');

inside of isEmpty function is not absolute correct. For example, string with blanks between   will be interpret as not empty. It seems to me the line should be better fixed to something like

o = o.replace(/(\s| |&\#160;)+/gi, '');

The line 525: 'i' is already defined.

The line 535: 'j' is already defined and 'lj' is already defined.

Lines 512, 529, 534, 536, 543, 544, 546, 547, 548, 551 should be better written in dot notation of ['data'] etc.

Lines 166, 171, 176, 180, 184, 190 has following error “Function statements cannot be placed in blocks. Use a function expression or move the statement to the top of the outer function”.

Lines 490, 566, 598 and 605 have missing semicolon.

-------------------------------------------------------------------

All files from the i18n directory contains line like

S: function (j) {return j < 11 || j > 13 ? ['st', 'nd', 'rd', 'th'][Math.min((j - 1) % 10, 3)] : 'th'},

have missing semicolon.

 

Best regards
Oleg