Skip to content

Commit

Permalink
fix(range): consistently convert min & max to numbers (#4587)
Browse files Browse the repository at this point in the history
* fix(range): consistently convert min & max to numbers

Basically we had two methods to turn a number (or string) into a precision:

1. Math.floor(min * pow) / pow
2. Number(Number(v).toFixed(precision))

these behave different for precision 0 & a number over .5, let's say 1.5 turns into 1 for method 1, and into 2 for method 2. This is a bug (not caught by most of our tests, notably the ones i had to change) and causes the range to change to a number it's not supposed to be. Pay special attention to the new `toPrecision` utility, and the places it was used.

At the same time I also converted to TypeScript, but not the components themselves, since that was too much work.

typescript for tests is expected to fail until algolia/algoliasearch-client-javascript#1229 is released (since now certain parts aren't random objects, but rather search responses)

The version for the helper is bumped for better typescript support as well.

update types

requires algolia/algoliasearch-client-javascript#1234

update client for fix in typing for tests

update assertions in changed tests

clarify why behaviour is like it is

* address feedback

* integrate fix from the helper avoiding workaround here
  • Loading branch information
Haroenv authored Dec 2, 2020
1 parent a54d3c7 commit ccf159e
Show file tree
Hide file tree
Showing 15 changed files with 2,670 additions and 2,230 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ module.exports = {
'instant_search',
'instant_search_movies',
'free_shipping',
'facets_stats',
'^EXPERIMENTAL_',
],
},
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
],
"dependencies": {
"@types/googlemaps": "^3.39.6",
"algoliasearch-helper": "^3.2.2",
"algoliasearch-helper": "^3.3.3",
"classnames": "^2.2.5",
"events": "^1.1.0",
"hogan.js": "^3.0.2",
Expand All @@ -62,7 +62,7 @@
"qs": "^6.5.1"
},
"devDependencies": {
"@algolia/client-search": "4.3.1",
"@algolia/client-search": "4.8.2",
"@babel/cli": "7.8.3",
"@babel/core": "7.8.3",
"@babel/plugin-proposal-class-properties": "7.8.3",
Expand Down Expand Up @@ -92,7 +92,7 @@
"@wdio/selenium-standalone-service": "5.16.5",
"@wdio/spec-reporter": "5.16.5",
"@wdio/static-server-service": "5.16.5",
"algoliasearch": "4.3.1",
"algoliasearch": "4.8.2",
"algoliasearch-v3": "npm:algoliasearch@3",
"babel-eslint": "10.0.3",
"babel-jest": "24.9.0",
Expand Down
8 changes: 8 additions & 0 deletions src/connectors/hits/__tests__/connectHits-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/hits/js/#co
name: 'hello',
_highlightResult: {
name: {
matchLevel: 'full' as const,
matchedWords: [],
value: `he${TAG_PLACEHOLDER.highlightPreTag}llo${TAG_PLACEHOLDER.highlightPostTag}`,
},
},
Expand All @@ -361,6 +363,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/hits/js/#co
name: 'halloween',
_highlightResult: {
name: {
matchLevel: 'full' as const,
matchedWords: [],
value: `ha${TAG_PLACEHOLDER.highlightPreTag}llo${TAG_PLACEHOLDER.highlightPostTag}ween`,
},
},
Expand All @@ -385,6 +389,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/hits/js/#co
name: 'hello',
_highlightResult: {
name: {
matchLevel: 'full',
matchedWords: [],
value: 'HE<MARK>LLO</MARK>',
},
},
Expand All @@ -394,6 +400,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/hits/js/#co
name: 'halloween',
_highlightResult: {
name: {
matchLevel: 'full',
matchedWords: [],
value: 'HA<MARK>LLO</MARK>WEEN',
},
},
Expand Down
14 changes: 14 additions & 0 deletions src/connectors/infinite-hits/__tests__/connectInfiniteHits-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,11 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi

const hits = [
{
foobar: '<script>foobar</script>',
_highlightResult: {
foobar: {
matchLevel: 'full' as const,
matchedWords: [],
value: `<script>${TAG_PLACEHOLDER.highlightPreTag}foobar${TAG_PLACEHOLDER.highlightPostTag}</script>`,
},
},
Expand All @@ -448,8 +451,11 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi

const escapedHits = [
{
foobar: '<script>foobar</script>',
_highlightResult: {
foobar: {
matchLevel: 'full',
matchedWords: [],
value: '&lt;script&gt;<mark>foobar</mark>&lt;/script&gt;',
},
},
Expand Down Expand Up @@ -555,6 +561,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
name: 'hello',
_highlightResult: {
name: {
matchLevel: 'full' as const,
matchedWords: [],
value: `he${TAG_PLACEHOLDER.highlightPreTag}llo${TAG_PLACEHOLDER.highlightPostTag}`,
},
},
Expand All @@ -564,6 +572,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
name: 'halloween',
_highlightResult: {
name: {
matchLevel: 'full' as const,
matchedWords: [],
value: `ha${TAG_PLACEHOLDER.highlightPreTag}llo${TAG_PLACEHOLDER.highlightPostTag}ween`,
},
},
Expand Down Expand Up @@ -591,6 +601,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
name: 'hello',
_highlightResult: {
name: {
matchLevel: 'full',
matchedWords: [],
value: 'HE<MARK>LLO</MARK>',
},
},
Expand All @@ -600,6 +612,8 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/infinite-hi
name: 'halloween',
_highlightResult: {
name: {
matchLevel: 'full',
matchedWords: [],
value: 'HA<MARK>LLO</MARK>WEEN',
},
},
Expand Down
3 changes: 2 additions & 1 deletion src/connectors/numeric-menu/connectNumericMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '../../lib/utils';
import { Connector, CreateURL, TransformItems } from '../../types';
import { SearchParameters } from 'algoliasearch-helper';
import { InsightsEvent } from '../../middlewares';

const withUsage = createDocumentationMessageGenerator({
name: 'numeric-menu',
Expand Down Expand Up @@ -103,7 +104,7 @@ export type NumericMenuConnector = Connector<
const $$type = 'ais.numericMenu';

const createSendEvent = ({ instantSearchInstance, helper, attribute }) => (
...args
...args: [InsightsEvent] | [string, string, string?]
) => {
if (args.length === 1) {
instantSearchInstance.sendEventToInsights(args[0]);
Expand Down
Loading

0 comments on commit ccf159e

Please sign in to comment.