refactor(lodash): remove startsWith #690

Haroenv · 2019-05-06T16:12:46Z

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

samouss · 2019-05-09T12:44:22Z

src/functions/formatSort.js

@@ -14,7 +13,7 @@ module.exports = function formatSort(sortBy, defaults) {
      var sortInstructions = sortInstruction.split(':');
      if (defaults && sortInstructions.length === 1) {
        var similarDefault = find(defaults, function(predicate) {
-          return startsWith(predicate, sortInstruction[0]);
+          return predicate[0] === sortInstruction[0];


On a single letter? Really? You don't think it's a mistake? Hopefully we don't have tokens that start two times with the same letter, otherwise that would break the implementation. It would have made more sense to use sortInstructions with an S rather than sortInstruction. WDYT?

I was doubting about this too, but forgot when merging, let's rethink this

@samouss

This was noticed in #690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO)

@samouss

This was noticed in #690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO)

@samouss

* fix(sortBy): compare whole prefix instead of first character This was noticed in #690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO) * test(formatSort): add UT * refactor: change conditions to lower nesting * clearer flow (slightly slower, but clearer)

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

@samouss

* fix(sortBy): compare whole prefix instead of first character This was noticed in #690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO) * test(formatSort): add UT * refactor: change conditions to lower nesting * clearer flow (slightly slower, but clearer)

* fix(merge): change implementation (#716) 29c2138 * chore: mention remaining changes f756947 * chore(deps): update circleci/node:8.15.1 docker digest to ef1a0c4 (#715) f4dab89 * chore(lodash): remove _.omit (#655) 7db8b4c * chore(lodash): remove lodash from dependency list (#705) a63ec6b, closes #552 * chore(release): allow canary release (#712) 3eb087c * chore(removeHierarchicalFacetRefinement): remove error if not refined (#747) e68ecfe * chore: merge develop into master (#750) 53c7c62 * chore: remove Bower support (#711) e7518e2 * chore(ts): add methods d051f79 * chore(ts): add ruleContexts to SP 4bdc4a4 * chore(ts): add searchOnlyWithDerivedHelpers (#739) c6aa31b * chore(ts): publish definition file too 1813462 * chore(TS): isNumericRefined has only one required argument cac8fc3 * chore(warn): remove unused function (lodash.bind) (#682) c59b7e3 * chore: update release command 0989880 * docs: add link to wiki 3598f2a * docs: update event signature (#701) bbe4634 * docs(migration): mention changed methods (#732) f95e680 * docs(next): add migration guide 8430137 * docs(serverUsed): mention getRankingInfo (#706) f258c2a, closes #500 * feat: implement dedicated reset page method (#673) 666501e * feat(getState): remove filter option (#707) ac52791 * feat(getState): remove getState (#708) 7de698c * feat(requestBuilder): prevent needless extra requests for empty refinements (#737) db0a392 * feat(search): allow the search only with Derived Helpers (#704) aa128fc * feat(SearchParameters): avoid undefined values (#703) 9757e0a * feat(typescript): move typings inline (#719) a12272e * fix(defaults): remove const 48a0c48 * fix(errors): remove isRefined (#731) 5761885 * fix(getConjunctiveRefinements): no error when requested facet is not conjunctive (#724) cf852e7 * fix(getDisjunctiveRefinements): remove error (#725) 211e390 * fix(getExcludeRefinements): replace error by default value (#726) 9d7ae87 * fix(getFacetStats): remove error (#721) 96b6ec8 * fix(getFacetValues): don't throw error when there's no facet (#720) e15e39e * fix(getHierarchicalFacetBreadcrumb): don't throw an error (#723) 40e1d61 * fix(isDisjunctiveFacetRefined): return false if not in disjunctiveFacets (#729) 13ec09b * fix(isExcludeRefined): remove error in favor of false (#728) 3f0ab6b * fix(isFacetRefined): return false if facet isn't declared (#727) 7151f56 * fix(isHierarchicalFacetRefined): return false if refinement isn't a facet (#730) 89fa010 * fix(lodash/intersection): replace with custom implementation (#718) 00dfb4e * fix(removeXFacet): make sure this fully removes empty arrays (#743) ea5a22a * fix(results): remove lodash looping over objects (#648) bb025c2, closes #258 * fix(sortBy): compare whole prefix instead of first character (#702) b85fb50, closes #702 * fix(toggleRefinement): keep an empty array when clearing (#738) 5b3fc11 * fix(types): add state.removeNumericRefinement (#742) e58c24a * refactor(addAgent): remove duplicate code (#657) d023efd * refactor(error): use object vs list of arguments (#700) 722eceb * refactor(lodash): compact (#689) 284efa5 * refactor(lodash): forOwn (#697) eae367a * refactor(lodash): intersection (#696) 25822a5 * refactor(lodash): map & trim (#679) 357fcb7 * refactor(lodash): merge (#694) 92bced4 * refactor(lodash): partial & partialRight (#693) 7ceea2f * refactor(lodash): remove filter (#685) 249d2e6 * refactor(lodash): remove flatten (#695) 9da0e08 * refactor(lodash): remove forEach (#674) 8c93765 * refactor(lodash): remove reduce (#678) 7907805 * refactor(lodash): remove simple functions (#656) c309ffa * refactor(lodash): remove startsWith (#690) bb00933 * refactor(lodash): replace defaults with pure alternative (#692) ee07132 * refactor(lodash): replace find & findIndex (#687) 92e7c23 * refactor(lodash): replace orderBy (#698) bb2b31e * refactor(lodash): sumBy (#688) a538bd9 * refactor(result): use object vs list of arguments (#699) 643f18e * refactor(search): emit object (#683) 46c7d7d * refactor(searchForFacetValues): use object vs list of arguments (#684) ab8e9c5 * refactor: remove getQueryParameter (#713) d9dfac4 * refactor(searchOnce): use object vs list of arguments (#681) 42b40d3 * refactor(SearchParameters): removes default values (#670) b15696b * refactor(url): remove url helpers (#652) 52e22f4 * test(sffv): no longer test impossible responses (#686) fd878e8 ### BREAKING CHANGE * getState(filters) is replaced my manually filtering the returned object * removed helper.isRefined, use helper.hasRefinements instead * SearchParameters.filter is removed * doc(filter): remove reference * use helper.state instead of helper.getState()

) The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

@samouss

…/algoliasearch-helper-js#702) * fix(sortBy): compare whole prefix instead of first character This was noticed in algolia/algoliasearch-helper-js#690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO) * test(formatSort): add UT * refactor: change conditions to lower nesting * clearer flow (slightly slower, but clearer)

refactor(lodash): remove startsWith

ec6b8ec

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

Haroenv requested a review from a team May 6, 2019 16:12

ghost requested review from eunjae-lee and yannickcr and removed request for a team May 6, 2019 16:12

yannickcr approved these changes May 6, 2019

View reviewed changes

Haroenv merged commit 468ba6f into develop May 6, 2019

Haroenv deleted the lodash/startsWith branch May 7, 2019 07:38

Haroenv added a commit that referenced this pull request May 7, 2019

refactor(lodash): remove startsWith (#690)

20dd0f0

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

Haroenv added a commit that referenced this pull request May 7, 2019

refactor(lodash): remove startsWith (#690)

14ba235

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

samouss pushed a commit that referenced this pull request May 9, 2019

refactor(lodash): remove startsWith (#690)

ff5a09c

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

samouss reviewed May 9, 2019

View reviewed changes

Haroenv added a commit that referenced this pull request May 9, 2019

fix(sortBy): compare whole prefix instead of first character

21bf852

This was noticed in #690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO)

Haroenv mentioned this pull request May 9, 2019

fix(sortBy): compare whole prefix instead of first character #702

Merged

Haroenv added a commit that referenced this pull request May 10, 2019

fix(sortBy): compare whole prefix instead of first character

b1fd5b8

This was noticed in #690 (comment) by @samouss, so I took the time to rewrite this function a little bit to be more clear (IMO)

Haroenv added a commit that referenced this pull request Nov 18, 2019

refactor(lodash): remove startsWith (#690)

bb00933

The comparison is a single letter, and the first part is also a string (because it's the value from find), meaning that they can simply be compared I think

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(lodash): remove startsWith #690

refactor(lodash): remove startsWith #690

Haroenv commented May 6, 2019

samouss May 9, 2019 •

edited

Loading

Haroenv May 9, 2019

refactor(lodash): remove startsWith #690

refactor(lodash): remove startsWith #690

Conversation

Haroenv commented May 6, 2019

samouss May 9, 2019 • edited Loading

Choose a reason for hiding this comment

Haroenv May 9, 2019

Choose a reason for hiding this comment

samouss May 9, 2019 •

edited

Loading