Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Declare vars when used, streamline code #1588

Merged
merged 5 commits into from
Dec 24, 2020
Merged

refactor: Declare vars when used, streamline code #1588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6a741a
refactor: Remove unnecessary branches
fb55 Dec 23, 2020
bf14d26
docs: Improve comments
fb55 Dec 23, 2020
107faf9
refactor(vars): Declare vars when used
fb55 Dec 24, 2020
c272cd6
chore(lint): Disallow shadowing, use before define
fb55 Dec 24, 2020
e41b84a
Replace unnecessary array
fb55 Dec 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"curly": [2, "multi-line"],
"one-var": [2, "never"],
"no-else-return": 2,
"no-shadow": 2,
"no-use-before-define": [2, "nofunc"],

"jsdoc/check-param-names": 2,
"jsdoc/check-tag-names": 2,
Expand Down
18 changes: 6 additions & 12 deletions benchmark/suite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ Suites.prototype.cheerioOnly = function () {
};

Suites.prototype.add = function (name, fileName, options) {
var markup;
var suite;
if (!filterRe.test(name)) {
return;
}
markup = fs.readFileSync(path.join(documentDir, fileName), 'utf8');
suite = new Benchmark.Suite(name);
var markup = fs.readFileSync(path.join(documentDir, fileName), 'utf8');
var suite = new Benchmark.Suite(name);

suite.on('start', function () {
console.log('Test: ' + name + ' (file: ' + fileName + ')');
Expand Down Expand Up @@ -70,10 +68,8 @@ Suites.prototype._benchJsDom = function (suite, markup, options) {

jQueryScript.runInContext(dom.getInternalVMContext());

var setupData;
if (options.setup) {
setupData = options.setup.call(null, dom.window.$);
}
var setupData = options.setup && options.setup.call(null, dom.window.$);

suite.add('jsdom', function () {
testFn.call(null, dom.window.$, setupData);
});
Expand All @@ -83,10 +79,8 @@ Suites.prototype._benchJsDom = function (suite, markup, options) {
Suites.prototype._benchCheerio = function (suite, markup, options) {
var $ = cheerio.load(markup);
var testFn = options.test;
var setupData;
if (options.setup) {
setupData = options.setup.call(null, $);
}
var setupData = options.setup && options.setup.call(null, $);

suite.add('cheerio', function () {
testFn.call(null, $, setupData);
});
Expand Down
132 changes: 51 additions & 81 deletions lib/api/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var rboolean = /^(?:autofocus|autoplay|async|checked|controls|defer|disabled|hid
// Matches strings that look like JSON objects or arrays
var rbrace = /^(?:\{[\w\W]*\}|\[[\w\W]*\])$/;

var getAttr = function (elem, name) {
function getAttr(elem, name) {
if (!elem || !isTag(elem)) return;

if (!elem.attribs) {
Expand Down Expand Up @@ -55,15 +55,15 @@ var getAttr = function (elem, name) {
) {
return 'on';
}
};
}

var setAttr = function (el, name, value) {
function setAttr(el, name, value) {
if (value === null) {
removeAttribute(el, name);
} else {
el.attribs[name] = value + '';
}
};
}

/**
* Method for getting and setting attributes. Gets the attribute value for only
Expand Down Expand Up @@ -109,23 +109,23 @@ exports.attr = function (name, value) {
return getAttr(this[0], name);
};

var getProp = function (el, name) {
function getProp(el, name) {
if (!el || !isTag(el)) return;

return name in el
? el[name]
: rboolean.test(name)
? getAttr(el, name) !== undefined
: getAttr(el, name);
};
}

var setProp = function (el, name, value) {
function setProp(el, name, value) {
if (name in el) {
el[name] = value;
} else {
setAttr(el, name, rboolean.test(name) ? (value ? '' : null) : value);
}
};
}

/**
* Method for getting and setting properties. Gets the property value for only
Expand All @@ -145,21 +145,19 @@ var setProp = function (el, name, value) {
* @see {@link http://api.jquery.com/prop/}
*/
exports.prop = function (name, value) {
var i = 0;
var property;

if (typeof name === 'string' && value === undefined) {
switch (name) {
case 'style':
property = this.css();

Object.keys(property).forEach(function (p) {
property[i++] = p;
case 'style': {
var property = this.css();
var keys = Object.keys(property);
keys.forEach(function (p, i) {
property[i] = p;
});

property.length = i;
property.length = keys.length;

break;
}
case 'tagName':
case 'nodeName':
property = this[0].name.toUpperCase();
Expand Down Expand Up @@ -199,7 +197,7 @@ exports.prop = function (name, value) {
}
};

var setData = function (el, name, value) {
function setData(el, name, value) {
if (!el.data) {
el.data = {};
}
Expand All @@ -208,21 +206,16 @@ var setData = function (el, name, value) {
if (typeof name === 'string' && value !== undefined) {
el.data[name] = value;
}
};
}

// Read the specified attribute from the equivalent HTML5 `data-*` attribute,
// and (if present) cache the value in the node's internal data store. If no
// attribute name is specified, read *all* HTML5 `data-*` attributes in this
// manner.
var readData = function (el, name) {
function readData(el, name) {
var readAll = arguments.length === 1;
var domNames;
var domName;
var jsNames;
var jsName;
var value;
var idx;
var length;

if (readAll) {
domNames = Object.keys(el.attribs).filter(function (attrName) {
Expand All @@ -236,9 +229,10 @@ var readData = function (el, name) {
jsNames = [name];
}

for (idx = 0, length = domNames.length; idx < length; ++idx) {
domName = domNames[idx];
jsName = jsNames[idx];
for (var idx = 0; idx < domNames.length; ++idx) {
var domName = domNames[idx];
var jsName = jsNames[idx];
var value;
if (hasOwn.call(el.attribs, domName) && !hasOwn.call(el.data, jsName)) {
value = el.attribs[domName];

Expand All @@ -259,7 +253,7 @@ var readData = function (el, name) {
}

return readAll ? el.data : value;
};
}

/**
* Method for getting and setting data attributes. Gets or sets the data
Expand Down Expand Up @@ -334,20 +328,11 @@ exports.val = function (value) {
switch (element.name) {
case 'textarea':
return this.text(value);
case 'input':
if (querying) {
return this.attr('value');
}

this.attr('value', value);
return this;

case 'select':
case 'select': {
var option = this.find('option:selected');
var returnValue;
if (option === undefined) return undefined;
if (!option) return undefined;
if (!querying) {
if (!hasOwn.call(this.attr(), 'multiple') && typeof value == 'object') {
if (this.attr('multiple') == null && typeof value == 'object') {
return this;
}
if (typeof value != 'object') {
Expand All @@ -359,20 +344,16 @@ exports.val = function (value) {
}
return this;
}
returnValue = option.attr('value');
if (hasOwn.call(this.attr(), 'multiple')) {
returnValue = [];
domEach(option, function (__, el) {
returnValue.push(getAttr(el, 'value'));
});
}
return returnValue;

return this.attr('multiple')
? option.toArray().map(function (el) {
return getAttr(el, 'value');
})
: option.attr('value');
}
case 'input':
case 'option':
if (!querying) {
this.attr('value', value);
return this;
}
return this.attr('value');
return querying ? this.attr('value') : this.attr('value', value);
}
};

Expand All @@ -383,11 +364,11 @@ exports.val = function (value) {
* @param {node} elem - Node to remove attribute from.
* @param {string} name - Name of the attribute to remove.
*/
var removeAttribute = function (elem, name) {
function removeAttribute(elem, name) {
if (!elem.attribs || !hasOwn.call(elem.attribs, name)) return;

delete elem.attribs[name];
};
}

/**
* Splits a space-separated list of names to individual
Expand All @@ -396,9 +377,9 @@ var removeAttribute = function (elem, name) {
* @param {string} names - Names to split.
* @returns {string[]} - Split names.
*/
var splitNames = function (names) {
function splitNames(names) {
return names ? names.trim().split(rspace) : [];
};
}

/**
* Method for removing attributes by `name`.
Expand Down Expand Up @@ -443,19 +424,18 @@ exports.removeAttr = function (name) {
* //=> true
*
* @param {string} className - Name of the class.
* @returns {boolean}
*
* @see {@link http://api.jquery.com/hasClass/}
*/
exports.hasClass = function (className) {
return this.toArray().some(function (elem) {
var attrs = elem.attribs;
var clazz = attrs && attrs['class'];
var clazz = elem.attribs && elem.attribs['class'];
var idx = -1;
var end;

if (clazz && className.length) {
while ((idx = clazz.indexOf(className, idx + 1)) > -1) {
end = idx + className.length;
var end = idx + className.length;

if (
(idx === 0 || rspace.test(clazz[idx - 1])) &&
Expand Down Expand Up @@ -505,17 +485,14 @@ exports.addClass = function (value) {

// If we don't already have classes
var className = getAttr(this[i], 'class');
var numClasses;
var setClass;

if (!className) {
setAttr(this[i], 'class', classNames.join(' ').trim());
} else {
setClass = ' ' + className + ' ';
numClasses = classNames.length;
var setClass = ' ' + className + ' ';

// Check if class already exists
for (var j = 0; j < numClasses; j++) {
for (var j = 0; j < classNames.length; j++) {
var appendClass = classNames[j] + ' ';
if (setClass.indexOf(' ' + appendClass) < 0) setClass += appendClass;
}
Expand Down Expand Up @@ -544,10 +521,6 @@ exports.addClass = function (value) {
* @see {@link http://api.jquery.com/removeClass/}
*/
exports.removeClass = function (value) {
var classes;
var numClasses;
var removeAll;

// Handle if value is a function
if (typeof value === 'function') {
return domEach(this, function (i, el) {
Expand All @@ -558,9 +531,9 @@ exports.removeClass = function (value) {
});
}

classes = splitNames(value);
numClasses = classes.length;
removeAll = arguments.length === 0;
var classes = splitNames(value);
var numClasses = classes.length;
var removeAll = arguments.length === 0;

return domEach(this, function (i, el) {
if (!isTag(el)) return;
Expand All @@ -570,11 +543,10 @@ exports.removeClass = function (value) {
el.attribs.class = '';
} else {
var elClasses = splitNames(el.attribs.class);
var index;
var changed;
var changed = false;

for (var j = 0; j < numClasses; j++) {
index = elClasses.indexOf(classes[j]);
var index = elClasses.indexOf(classes[j]);

if (index >= 0) {
elClasses.splice(index, 1);
Expand Down Expand Up @@ -629,19 +601,17 @@ exports.toggleClass = function (value, stateVal) {
var numClasses = classNames.length;
var state = typeof stateVal === 'boolean' ? (stateVal ? 1 : -1) : 0;
var numElements = this.length;
var elementClasses;
var index;

for (var i = 0; i < numElements; i++) {
// If selected element isn't a tag, move on
if (!isTag(this[i])) continue;

elementClasses = splitNames(this[i].attribs.class);
var elementClasses = splitNames(this[i].attribs.class);

// Check if class already exists
for (var j = 0; j < numClasses; j++) {
// Check if the class name is currently defined
index = elementClasses.indexOf(classNames[j]);
var index = elementClasses.indexOf(classNames[j]);

// Add if stateValue === true or we are toggling and there is no value
if (state >= 0 && index < 0) {
Expand Down
3 changes: 1 addition & 2 deletions lib/api/forms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ exports.serialize = function () {
exports.serializeArray = function () {
// Resolve all form elements from either forms or collections of form elements
var Cheerio = this.constructor;
return this.map(function () {
var elem = this;
return this.map(function (i, elem) {
var $elem = Cheerio(elem);
if (elem.name === 'form') {
return $elem.find(submittableSelector).toArray();
Expand Down