From fa98e21cc6f65483e1b444073ad1788d9b3de99b Mon Sep 17 00:00:00 2001 From: Fabien Motte Date: Wed, 12 Apr 2023 16:23:57 +0200 Subject: [PATCH] fix(insights): only `init` if `version` allows it (#5529) Co-authored-by: Dhaya <154633+dhayab@users.noreply.github.com> --- packages/instantsearch.js/package.json | 2 +- .../__tests__/createInsightsMiddleware.ts | 88 +++++++++++++++---- .../middlewares/createInsightsMiddleware.ts | 23 +++-- tests/mocks/createInsightsClient.ts | 23 +++-- yarn.lock | 8 +- 5 files changed, 110 insertions(+), 34 deletions(-) diff --git a/packages/instantsearch.js/package.json b/packages/instantsearch.js/package.json index 2460df054c..a3f7c856e8 100644 --- a/packages/instantsearch.js/package.json +++ b/packages/instantsearch.js/package.json @@ -38,7 +38,7 @@ "htm": "^3.0.0", "preact": "^10.10.0", "qs": "^6.5.1 < 6.10", - "search-insights": "^2.1.0" + "search-insights": "^2.4.0" }, "peerDependencies": { "algoliasearch": ">= 3.1 < 6" diff --git a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts index 904c0eadc4..14d6484244 100644 --- a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts @@ -263,6 +263,7 @@ describe('insights', () => { it('notifies when the script fails to be added', () => { const { instantSearchInstance } = createTestEnvironment(); + /* eslint-disable deprecation/deprecation */ const createElement = document.createElement; document.createElement = () => { throw new Error('error'); @@ -277,6 +278,7 @@ describe('insights', () => { instantSearchInstance.use(createInsightsMiddleware()); document.createElement = createElement; + /* eslint-enable deprecation/deprecation */ }); it('notifies when the script fails to load', () => { @@ -309,25 +311,11 @@ describe('insights', () => { apiKey: 'myApiKey', appId: 'myAppId', region: 'de', + partial: true, useCookie: false, }); }); - it('throws when search client does not have credentials', () => { - const { insightsClient } = createInsights(); - const instantSearchInstance = createInstantSearch({ - // @ts-expect-error fake client - client: { search: () => {} }, - }); - expect(() => - createInsightsMiddleware({ - insightsClient, - })({ instantSearchInstance }) - ).toThrowErrorMatchingInlineSnapshot( - `"apiKey is missing, please provide it so we can authenticate the application"` - ); - }); - it('warns when search client does not have credentials', () => { const { insightsClient } = createInsights(); const instantSearchInstance = createInstantSearch({ @@ -581,6 +569,74 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f ] `); }); + + it('does not call `init` when default middleware is used', () => { + const { instantSearchInstance, insightsClient } = createTestEnvironment({ + insights: true, + }); + + instantSearchInstance.use( + createInsightsMiddleware({ $$internal: true, insightsClient }) + ); + + expect(instantSearchInstance.middleware).toHaveLength(1); + expect(instantSearchInstance.middleware).toMatchInlineSnapshot(` + [ + { + "creator": [Function], + "instance": { + "$$internal": true, + "$$type": "ais.insights", + "onStateChange": [Function], + "started": [Function], + "subscribe": [Function], + "unsubscribe": [Function], + }, + }, + ] + `); + expect(insightsClient).not.toHaveBeenCalledWith('init', { + apiKey: 'myApiKey', + appId: 'myAppId', + partial: true, + useCookie: true, + }); + }); + + it('does call `init` when `initParams` are passed', () => { + const { instantSearchInstance, insightsClient } = createTestEnvironment(); + + instantSearchInstance.use( + createInsightsMiddleware({ + $$internal: true, + insightsClient, + insightsInitParams: { useCookie: false }, + }) + ); + + expect(instantSearchInstance.middleware).toHaveLength(1); + expect(instantSearchInstance.middleware).toMatchInlineSnapshot(` + [ + { + "creator": [Function], + "instance": { + "$$internal": true, + "$$type": "ais.insights", + "onStateChange": [Function], + "started": [Function], + "subscribe": [Function], + "unsubscribe": [Function], + }, + }, + ] + `); + expect(insightsClient).toHaveBeenCalledWith('init', { + apiKey: 'myApiKey', + appId: 'myAppId', + partial: true, + useCookie: false, + }); + }); }); describe('userToken', () => { @@ -768,7 +824,7 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f }, ]); - insightsClient('setUserToken', undefined); + insightsClient('setUserToken', ''); await wait(0); expect(instantSearchInstance.client.search).toHaveBeenCalledTimes(2); diff --git a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts index 0ec8042cd8..dafeee264b 100644 --- a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts @@ -73,6 +73,7 @@ export function createInsightsMiddleware< if (!potentialInsightsClient) { window.AlgoliaAnalyticsObject = pointer; + if (!window[pointer]) { window[pointer] = (...args: any[]) => { if (!window[pointer].queue) { @@ -138,12 +139,18 @@ export function createInsightsMiddleware< // Otherwise, the `init` call might override it with anonymous user token. userTokenBeforeInit = userToken; }); - insightsClient('init', { - appId, - apiKey, - useCookie: true, - ...insightsInitParams, - }); + + // Only `init` if the `insightsInitParams` option is passed or + // if the `insightsClient` version doesn't supports optional `init` calling. + if (insightsInitParams || !isModernInsightsClient(insightsClient)) { + insightsClient('init', { + appId, + apiKey, + useCookie: true, + partial: true, + ...insightsInitParams, + }); + } let initialParameters: PlainSearchParameters; let helper: AlgoliaSearchHelper; @@ -306,6 +313,10 @@ See documentation: https://www.algolia.com/doc/guides/building-search-ui/going-f }; } +/** + * Determines if a given insights `client` supports the optional call to `init` + * and the ability to set credentials via extra parameters when sending events. + */ function isModernInsightsClient(client: InsightsClientWithGlobals): boolean { const [major, minor] = (client.version || '').split('.').map(Number); diff --git a/tests/mocks/createInsightsClient.ts b/tests/mocks/createInsightsClient.ts index 849486ed42..73d8571e3a 100644 --- a/tests/mocks/createInsightsClient.ts +++ b/tests/mocks/createInsightsClient.ts @@ -5,6 +5,15 @@ import { } from 'search-insights'; import type { InsightsClient } from 'instantsearch.js'; +import { castToJestMock } from '@instantsearch/testutils/castToJestMock'; + +/** + * Tests that rely on this mock interface have side effects caused by + * the import of search-insights. The following code deletes those side effects. + */ +try { + delete window.AlgoliaAnalyticsObject; +} catch (error) {} // eslint-disable-line no-empty export function createInsights({ forceVersion = '2.4.0', @@ -16,22 +25,22 @@ export function createInsights({ requestFn: jest.fn(), }) ); + const mockedInsightsClient = castToJestMock( + jest.fn(getFunctionalInterface(analytics)) as InsightsClient + ); if (forceVersion) { return { analytics, - insightsClient: Object.assign( - jest.fn(getFunctionalInterface(analytics)), - { - version: forceVersion, - } - ), + insightsClient: Object.assign(mockedInsightsClient, { + version: forceVersion, + }), }; } return { analytics, - insightsClient: jest.fn(getFunctionalInterface(analytics)), + insightsClient: mockedInsightsClient, }; } diff --git a/yarn.lock b/yarn.lock index b279400cf7..adcbaae0cf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -30442,10 +30442,10 @@ scule@^0.2.1: resolved "https://registry.yarnpkg.com/scule/-/scule-0.2.1.tgz#0c1dc847b18e07219ae9a3832f2f83224e2079dc" integrity sha512-M9gnWtn3J0W+UhJOHmBxBTwv8mZCan5i1Himp60t6vvZcor0wr+IM0URKmIglsWJ7bRujNAVVN77fp+uZaWoKg== -search-insights@^2.1.0: - version "2.1.0" - resolved "https://registry.yarnpkg.com/search-insights/-/search-insights-2.1.0.tgz#42822fa325062ec5bf050c5f8cad58f0c5b21310" - integrity sha512-mWIVsZ5igQnlM2tC0HJCtGWKQcBhLhkasZoZZS2exUfZPpRowVWEvyyYTWU3M0rUKWlWLWFCYvJ3HGTSSCsOcw== +search-insights@^2.4.0: + version "2.4.0" + resolved "https://registry.yarnpkg.com/search-insights/-/search-insights-2.4.0.tgz#7b6b19527dd186b459f6e2dd7e01f7a3de96ffe7" + integrity sha512-AqXxWFEIZTfOf0brQLvoAZcotrVX0xR/VoPCzBxsTZF/yO+izIH1eFCtKizR/dGI8NCMOTdc7l90hAOI68dBbg== section-iterator@^2.0.0: version "2.0.0"