From bc69429cd0bebcb0779a6c9b5c94fe7e21b2284e Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Thu, 16 Mar 2023 10:10:53 +0100 Subject: [PATCH] fix(insights): user-defined send/bindEvent overrides internal click (#5527) * fix(insights): prevent duplicate events in sendEvent, we mark internal events (click and conversion), and prevent them being sent if there was a custom click right before. Note that this doesn't yet work with bindEvent, as it's higher in the bubbling tree * fix(internal): bail out of default click if element has a bindEvent parent * test * fix: only prevent next clicks after a regular click * bundlesize * test(infiniteHits): add sendEvent tests same idea as this test in hits-integration-test.ts * wip infinite hits * move bindEvent click listener people can't bind events higher anyway, so this is safe to do * refactor: move dedupe into middleware this allows the dedupe to work for both bindEvent and sendEvent in a single codebase. Tiny aside: this means that the "next internal click" is now global, meaning if you click on geo, then on hits, we'll see that as an internal click to prevent sending. This is an extreme edge case, as almost nowhere sends click events. * fix(events): move back inside connector this prevents a custom blocking an internal in a different widget * get rid of global cruft * refactor: use timers instead of order-based ignoring * test(common): insights clicks + view for hits + infinitehits * fix(react): send an internal event * PR feedback * internal --- bundlesize.config.json | 8 +- .../src/__tests__/common.test.tsx | 111 ++++ .../src/components/Hits/Hits.tsx | 7 +- .../components/Hits/__tests__/Hits.test.tsx | 2 +- .../__snapshots__/Hits-test.tsx.snap | 2 - .../components/InfiniteHits/InfiniteHits.tsx | 7 +- .../__tests__/InfiniteHits.test.tsx | 2 +- .../__tests__/insights-listener-test.tsx | 6 +- .../src/lib/utils/createSendEventForHits.ts | 136 ++-- .../hits/__tests__/hits-integration-test.ts | 52 +- .../infinite-hits-integration-test.ts | 152 ++++- .../src/__tests__/common.test.tsx | 116 +++- .../src/ui/Hits.tsx | 2 +- .../src/ui/InfiniteHits.tsx | 2 +- .../src/ui/__tests__/Hits.test.tsx | 2 +- .../src/ui/__tests__/InfiniteHits.test.tsx | 2 +- .../src/__tests__/common.test.js | 189 +++++- tests/common/index.ts | 1 + tests/common/widgets/hits/index.ts | 24 + tests/common/widgets/hits/insights.ts | 584 ++++++++++++++++++ tests/common/widgets/infinite-hits/index.ts | 6 +- .../common/widgets/infinite-hits/insights.ts | 584 ++++++++++++++++++ .../widgets/infinite-hits/optimistic-ui.ts | 34 +- 23 files changed, 1906 insertions(+), 125 deletions(-) create mode 100644 tests/common/widgets/hits/index.ts create mode 100644 tests/common/widgets/hits/insights.ts create mode 100644 tests/common/widgets/infinite-hits/insights.ts diff --git a/bundlesize.config.json b/bundlesize.config.json index 469564c6f5..27c887d214 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -26,11 +26,11 @@ }, { "path": "packages/react-instantsearch-hooks/dist/umd/ReactInstantSearchHooks.min.js", - "maxSize": "43 kB" + "maxSize": "43.25 kB" }, { "path": "packages/react-instantsearch-hooks-web/dist/umd/ReactInstantSearchHooksDOM.min.js", - "maxSize": "52.50 kB" + "maxSize": "52.75 kB" }, { "path": "packages/react-instantsearch-dom/dist/umd/ReactInstantSearchDOM.min.js", @@ -42,11 +42,11 @@ }, { "path": "packages/vue-instantsearch/vue2/umd/index.js", - "maxSize": "61.25 kB" + "maxSize": "61.50 kB" }, { "path": "packages/vue-instantsearch/vue3/umd/index.js", - "maxSize": "61.25 kB" + "maxSize": "61.75 kB" }, { "path": "packages/vue-instantsearch/vue2/cjs/index.js", diff --git a/packages/instantsearch.js/src/__tests__/common.test.tsx b/packages/instantsearch.js/src/__tests__/common.test.tsx index bc51c2ebe3..e193213942 100644 --- a/packages/instantsearch.js/src/__tests__/common.test.tsx +++ b/packages/instantsearch.js/src/__tests__/common.test.tsx @@ -8,6 +8,7 @@ import { createPaginationTests, createMenuTests, createInfiniteHitsTests, + createHitsTests, } from '@instantsearch/tests'; import instantsearch from '../index.es'; @@ -19,6 +20,8 @@ import { pagination, infiniteHits, searchBox, + hits, + index, } from '../widgets'; createHierarchicalMenuTests(({ instantSearchOptions, widgetParams }) => { @@ -120,9 +123,117 @@ createInfiniteHitsTests(({ instantSearchOptions, widgetParams }) => { container: document.body.appendChild(document.createElement('div')), }), infiniteHits({ + container: document.body.appendChild( + Object.assign(document.createElement('div'), { + id: 'main-hits', + }) + ), + templates: { + item: (hit, { html, sendEvent }) => + html`
+ ${hit.objectID} + + +
`, + }, + ...widgetParams, + }), + index({ indexName: 'nested' }).addWidgets([ + infiniteHits({ + container: document.body.appendChild( + Object.assign(document.createElement('div'), { + id: 'nested-hits', + }) + ), + templates: { + item: (hit, { html, sendEvent }) => + html`
+ ${hit.objectID} + +
`, + }, + }), + ]), + ]) + .on('error', () => { + /* + * prevent rethrowing InstantSearch errors, so tests can be asserted. + * IRL this isn't needed, as the error doesn't stop execution. + */ + }) + .start(); +}); + +createHitsTests(({ instantSearchOptions, widgetParams }) => { + instantsearch(instantSearchOptions) + .addWidgets([ + searchBox({ container: document.body.appendChild(document.createElement('div')), + }), + hits({ + container: document.body.appendChild( + Object.assign(document.createElement('div'), { + id: 'main-hits', + }) + ), + templates: { + item: (hit, { html, sendEvent }) => + html`
+ ${hit.objectID} + + +
`, + }, ...widgetParams, }), + index({ indexName: 'nested' }).addWidgets([ + hits({ + container: document.body.appendChild( + Object.assign(document.createElement('div'), { + id: 'nested-hits', + }) + ), + templates: { + item: (hit, { html, sendEvent }) => + html`
+ ${hit.objectID} + +
`, + }, + }), + ]), ]) .on('error', () => { /* diff --git a/packages/instantsearch.js/src/components/Hits/Hits.tsx b/packages/instantsearch.js/src/components/Hits/Hits.tsx index c4be9ae42f..cbad18527e 100644 --- a/packages/instantsearch.js/src/components/Hits/Hits.tsx +++ b/packages/instantsearch.js/src/components/Hits/Hits.tsx @@ -55,7 +55,7 @@ export default function Hits({ } return ( -
+
    {hits.map((hit, index) => (