From c78bd0f23b11a3d9dbb8e714ea69eeb20d844e82 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Fri, 10 May 2019 15:04:22 +0200 Subject: [PATCH] refactor(lodash): remove simple functions (#656) * isFunction * keys * isEmpty & isUndefined * isEqual * isNaN * Update src/SearchParameters/index.js Co-Authored-By: Haroenv * feedback from review * fix(params): harden conditions for empty refinements this used to be isEmpty, so here we use a similar, but more focused condition * chore(numeric): make helper function clearer * refactor: use created helper method * fix: don't use const * feat(objectHasKeys): return false for falsy (non-object) --- src/SearchParameters/RefinementList.js | 33 ++++---- src/SearchParameters/filterState.js | 12 +-- src/SearchParameters/index.js | 101 ++++++++++++++++--------- src/SearchResults/index.js | 4 +- src/algoliasearch.helper.js | 5 +- src/functions/objectHasKeys.js | 7 ++ src/functions/valToNumber.js | 2 +- 7 files changed, 100 insertions(+), 64 deletions(-) create mode 100644 src/functions/objectHasKeys.js diff --git a/src/SearchParameters/RefinementList.js b/src/SearchParameters/RefinementList.js index 9c39f3557..9afc2f77d 100644 --- a/src/SearchParameters/RefinementList.js +++ b/src/SearchParameters/RefinementList.js @@ -12,12 +12,9 @@ * @typedef {Object.} SearchParameters.refinementList.RefinementList */ -var isUndefined = require('lodash/isUndefined'); -var isFunction = require('lodash/isFunction'); -var isEmpty = require('lodash/isEmpty'); - var defaultsPure = require('../functions/defaultsPure'); var omit = require('../functions/omit'); +var objectHasKeys = require('../functions/objectHasKeys'); var lib = { /** @@ -54,7 +51,7 @@ var lib = { * @return {RefinementList} a new and updated refinement lst */ removeRefinement: function removeRefinement(refinementList, attribute, value) { - if (isUndefined(value)) { + if (value === undefined) { return lib.clearRefinement(refinementList, attribute); } @@ -72,7 +69,7 @@ var lib = { * @return {RefinementList} a new and updated list */ toggleRefinement: function toggleRefinement(refinementList, attribute, value) { - if (isUndefined(value)) throw new Error('toggleRefinement should be used with a value'); + if (value === undefined) throw new Error('toggleRefinement should be used with a value'); if (lib.isRefined(refinementList, attribute, value)) { return lib.removeRefinement(refinementList, attribute, value); @@ -92,13 +89,17 @@ var lib = { * @return {RefinementList} a new and updated refinement list */ clearRefinement: function clearRefinement(refinementList, attribute, refinementType) { - if (isUndefined(attribute)) { - if (isEmpty(refinementList)) return refinementList; + if (attribute === undefined) { + if (!objectHasKeys(refinementList)) { + return refinementList; + } return {}; } else if (typeof attribute === 'string') { - if (isEmpty(refinementList[attribute])) return refinementList; + if (!(refinementList[attribute] && refinementList[attribute].length > 0)) { + return refinementList; + } return omit(refinementList, attribute); - } else if (isFunction(attribute)) { + } else if (typeof attribute === 'function') { var hasChanged = false; var newRefinementList = Object.keys(refinementList).reduce(function(memo, key) { @@ -106,12 +107,14 @@ var lib = { var facetList = values.filter(function(value) { return !attribute(value, key, refinementType); }); - - if (!isEmpty(facetList)) { - if (facetList.length !== values.length) hasChanged = true; + if (facetList.length > 0) { + if (facetList.length !== values.length) { + hasChanged = true; + } memo[key] = facetList; + } else { + hasChanged = true; } - else hasChanged = true; return memo; }, {}); @@ -133,7 +136,7 @@ var lib = { var containsRefinements = !!refinementList[attribute] && refinementList[attribute].length > 0; - if (isUndefined(refinementValue) || !containsRefinements) { + if (refinementValue === undefined || !containsRefinements) { return containsRefinements; } diff --git a/src/SearchParameters/filterState.js b/src/SearchParameters/filterState.js index c3100f410..7efde8e40 100644 --- a/src/SearchParameters/filterState.js +++ b/src/SearchParameters/filterState.js @@ -1,6 +1,6 @@ 'use strict'; -var isEmpty = require('lodash/isEmpty'); +var objectHasKeys = require('../functions/objectHasKeys'); /** * @param {any[]} filters @@ -28,20 +28,20 @@ function filterState(state, filters) { } var numericRefinements = state.getNumericRefinements(attr); - if (!isEmpty(numericRefinements)) { + if (objectHasKeys(numericRefinements)) { if (!partialState.numericRefinements) partialState.numericRefinements = {}; partialState.numericRefinements[attr] = state.numericRefinements[attr]; } }); } else { - if (!isEmpty(state.numericRefinements)) { + if (objectHasKeys(state.numericRefinements)) { partialState.numericRefinements = state.numericRefinements; } - if (!isEmpty(state.facetsRefinements)) partialState.facetsRefinements = state.facetsRefinements; - if (!isEmpty(state.disjunctiveFacetsRefinements)) { + if (objectHasKeys(state.facetsRefinements)) partialState.facetsRefinements = state.facetsRefinements; + if (objectHasKeys(state.disjunctiveFacetsRefinements)) { partialState.disjunctiveFacetsRefinements = state.disjunctiveFacetsRefinements; } - if (!isEmpty(state.hierarchicalFacetsRefinements)) { + if (objectHasKeys(state.hierarchicalFacetsRefinements)) { partialState.hierarchicalFacetsRefinements = state.hierarchicalFacetsRefinements; } } diff --git a/src/SearchParameters/index.js b/src/SearchParameters/index.js index c46b9915c..40ed041a3 100644 --- a/src/SearchParameters/index.js +++ b/src/SearchParameters/index.js @@ -1,34 +1,47 @@ 'use strict'; -var keys = require('lodash/keys'); -var isNaN = require('lodash/isNaN'); -var isEmpty = require('lodash/isEmpty'); -var isEqual = require('lodash/isEqual'); -var isUndefined = require('lodash/isUndefined'); -var isFunction = require('lodash/isFunction'); - var merge = require('lodash/merge'); var defaultsPure = require('../functions/defaultsPure'); var find = require('../functions/find'); var valToNumber = require('../functions/valToNumber'); var omit = require('../functions/omit'); +var objectHasKeys = require('../functions/objectHasKeys'); var filterState = require('./filterState'); var RefinementList = require('./RefinementList'); /** - * like _.find but using _.isEqual to be able to use it + * isEqual, but only for numeric refinement values, possible values: + * - 5 + * - [5] + * - [[5]] + * - [[5,5],[4]] + */ +function isEqualNumericRefinement(a, b) { + if (Array.isArray(a) && Array.isArray(b)) { + return ( + a.length === b.length && + a.every(function(el, i) { + return isEqualNumericRefinement(b[i], el); + }) + ); + } + return a === b; +} + +/** + * like _.find but using deep equality to be able to use it * to find arrays. * @private - * @param {any[]} array array to search into - * @param {any} searchedValue the value we're looking for + * @param {any[]} array array to search into (elements are base or array of base) + * @param {any} searchedValue the value we're looking for (base or array of base) * @return {any} the searched value or undefined */ function findArray(array, searchedValue) { return find(array, function(currentValue) { - return isEqual(currentValue, searchedValue); + return isEqualNumericRefinement(currentValue, searchedValue); }); } @@ -195,7 +208,7 @@ function SearchParameters(newParameters) { * This doesn't contain any beta/hidden features. * @private */ -SearchParameters.PARAMETERS = keys(new SearchParameters()); +SearchParameters.PARAMETERS = Object.keys(new SearchParameters()); /** * @private @@ -226,6 +239,7 @@ SearchParameters._parseNumbers = function(partialState) { var value = partialState[k]; if (typeof value === 'string') { var parsedValue = parseFloat(value); + // global isNaN is ok to use here, value is only number or NaN numbers[k] = isNaN(parsedValue) ? value : parsedValue; } }); @@ -319,14 +333,19 @@ SearchParameters.validate = function(currentState, parameters) { 'an error, if it is not, you should first clear the tags with clearTags method.'); } - if (currentState.numericFilters && params.numericRefinements && !isEmpty(params.numericRefinements)) { + if ( + currentState.numericFilters && + params.numericRefinements && + objectHasKeys(params.numericRefinements) + ) { return new Error( "[Numeric filters] Can't switch from the advanced to the managed API. It" + - ' is probably an error, if this is really what you want, you have to first' + - ' clear the numeric filters.'); + ' is probably an error, if this is really what you want, you have to first' + + ' clear the numeric filters.' + ); } - if (!isEmpty(currentState.numericRefinements) && params.numericFilters) { + if (objectHasKeys(currentState.numericRefinements) && params.numericFilters) { return new Error( "[Numeric filters] Can't switch from the managed API to the advanced. It" + ' is probably an error, if this is really what you want, you have to first' + @@ -565,11 +584,16 @@ SearchParameters.prototype = { */ removeNumericRefinement: function(attribute, operator, paramValue) { if (paramValue !== undefined) { - var paramValueAsNumber = valToNumber(paramValue); - if (!this.isNumericRefined(attribute, operator, paramValueAsNumber)) return this; + if (!this.isNumericRefined(attribute, operator, paramValue)) { + return this; + } return this.setQueryParameters({ numericRefinements: this._clearNumericRefinements(function(value, key) { - return key === attribute && value.op === operator && isEqual(value.val, paramValueAsNumber); + return ( + key === attribute && + value.op === operator && + isEqualNumericRefinement(value.val, valToNumber(paramValue)) + ); }) }); } else if (operator !== undefined) { @@ -616,13 +640,17 @@ SearchParameters.prototype = { * @return {Object.} */ _clearNumericRefinements: function _clearNumericRefinements(attribute) { - if (isUndefined(attribute)) { - if (isEmpty(this.numericRefinements)) return this.numericRefinements; + if (attribute === undefined) { + if (!objectHasKeys(this.numericRefinements)) { + return this.numericRefinements; + } return {}; } else if (typeof attribute === 'string') { - if (isEmpty(this.numericRefinements[attribute])) return this.numericRefinements; + if (!objectHasKeys(this.numericRefinements[attribute])) { + return this.numericRefinements; + } return omit(this.numericRefinements, attribute); - } else if (isFunction(attribute)) { + } else if (typeof attribute === 'function') { var hasChanged = false; var numericRefinements = this.numericRefinements; var newNumericRefinements = Object.keys(numericRefinements).reduce(function(memo, key) { @@ -637,14 +665,16 @@ SearchParameters.prototype = { var predicateResult = attribute({val: value, op: operator}, key, 'numeric'); if (!predicateResult) outValues.push(value); }); - if (!isEmpty(outValues)) { + if (outValues.length > 0) { if (outValues.length !== values.length) hasChanged = true; operatorList[operator] = outValues; } else hasChanged = true; }); - if (!isEmpty(operatorList)) memo[key] = operatorList; + if (objectHasKeys(operatorList)) { + memo[key] = operatorList; + } return memo; }, {}); @@ -1179,21 +1209,22 @@ SearchParameters.prototype = { * @return {boolean} true if it is refined */ isNumericRefined: function isNumericRefined(attribute, operator, value) { - if (isUndefined(value) && isUndefined(operator)) { + if (value === undefined && operator === undefined) { return !!this.numericRefinements[attribute]; } - var isOperatorDefined = this.numericRefinements[attribute] && - !isUndefined(this.numericRefinements[attribute][operator]); + var isOperatorDefined = + this.numericRefinements[attribute] && + this.numericRefinements[attribute][operator] !== undefined; - if (isUndefined(value) || !isOperatorDefined) { + if (value === undefined || !isOperatorDefined) { return isOperatorDefined; } var parsedValue = valToNumber(value); - var isAttributeValueDefined = !isUndefined( - findArray(this.numericRefinements[attribute][operator], parsedValue) - ); + var isAttributeValueDefined = + findArray(this.numericRefinements[attribute][operator], parsedValue) !== + undefined; return isOperatorDefined && isAttributeValueDefined; }, @@ -1222,7 +1253,7 @@ SearchParameters.prototype = { } ); - return keys(this.disjunctiveFacetsRefinements) + return Object.keys(this.disjunctiveFacetsRefinements) .concat(disjunctiveNumericRefinedFacets) .concat(this.getRefinedHierarchicalFacets()); }, @@ -1330,9 +1361,7 @@ SearchParameters.prototype = { var parsedParams = SearchParameters._parseNumbers(params); return this.mutateMe(function mergeWith(newInstance) { - var ks = keys(params); - - ks.forEach(function(k) { + Object.keys(params).forEach(function(k) { newInstance[k] = parsedParams[k]; }); diff --git a/src/SearchResults/index.js b/src/SearchResults/index.js index eb1649bd1..dc5266669 100644 --- a/src/SearchResults/index.js +++ b/src/SearchResults/index.js @@ -2,8 +2,6 @@ var merge = require('lodash/merge'); -var isFunction = require('lodash/isFunction'); - var defaultsPure = require('../functions/defaultsPure'); var orderBy = require('../functions/orderBy'); var compact = require('../functions/compact'); @@ -706,7 +704,7 @@ SearchResults.prototype.getFacetValues = function(attribute, opts) { return recSort(function(hierarchicalFacetValues) { return orderBy(hierarchicalFacetValues, order[0], order[1]); }, facetValues); - } else if (isFunction(options.sortBy)) { + } else if (typeof options.sortBy === 'function') { if (Array.isArray(facetValues)) { return facetValues.sort(options.sortBy); } diff --git a/src/algoliasearch.helper.js b/src/algoliasearch.helper.js index bf70bd8f5..8fe8dc5be 100644 --- a/src/algoliasearch.helper.js +++ b/src/algoliasearch.helper.js @@ -7,8 +7,7 @@ var requestBuilder = require('./requestBuilder'); var events = require('events'); var inherits = require('./functions/inherits'); - -var isEmpty = require('lodash/isEmpty'); +var objectHasKeys = require('./functions/objectHasKeys'); var version = require('./version'); @@ -985,7 +984,7 @@ AlgoliaSearchHelper.prototype.isRefined = function(facet, value) { * */ AlgoliaSearchHelper.prototype.hasRefinements = function(attribute) { - if (!isEmpty(this.state.getNumericRefinements(attribute))) { + if (objectHasKeys(this.state.getNumericRefinements(attribute))) { return true; } else if (this.state.isConjunctiveFacet(attribute)) { return this.state.isFacetRefined(attribute); diff --git a/src/functions/objectHasKeys.js b/src/functions/objectHasKeys.js new file mode 100644 index 000000000..c7c44bb1a --- /dev/null +++ b/src/functions/objectHasKeys.js @@ -0,0 +1,7 @@ +'use strict'; + +function objectHasKeys(obj) { + return obj && Object.keys(obj).length > 0; +} + +module.exports = objectHasKeys; diff --git a/src/functions/valToNumber.js b/src/functions/valToNumber.js index bc5b54d4c..f92eb64ce 100644 --- a/src/functions/valToNumber.js +++ b/src/functions/valToNumber.js @@ -9,7 +9,7 @@ function valToNumber(v) { return v.map(valToNumber); } - throw new Error('The value should be a number, a parseable string or an array of those.'); + throw new Error('The value should be a number, a parsable string or an array of those.'); } module.exports = valToNumber;