fix(instantsearch.js): fix user token not being set in time for the first query #6377
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d039738:
|
Haroenv
reviewed
Oct 22, 2024
packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts
Outdated
Show resolved
Hide resolved
packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts
Outdated
Show resolved
Hide resolved
| @@ -287,12 +369,30 @@ export function createInsightsMiddleware< | |||
| queuedUserToken, | |||
| queuedAuthenticatedUserToken | |||
| ); | |||
| } else if (anonymousUserToken) { | |||
| setUserToken(anonymousUserToken, anonymousUserToken, undefined); | |||
There was a problem hiding this comment.
would like to see the setUserToken function simplified (can be separate pr) to not have three arguments
shaejaz
commented
Oct 22, 2024
packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts
Show resolved
Hide resolved
packages/instantsearch.js/src/lib/__tests__/InstantSearch-integration-test.ts
Show resolved
Hide resolved
Haroenv
reviewed
Oct 22, 2024
packages/instantsearch.js/src/lib/__tests__/InstantSearch-integration-test.ts
Show resolved
Hide resolved
packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts
Show resolved
Hide resolved
packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts
Show resolved
Hide resolved
packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts
Outdated
Show resolved
Hide resolved
Haroenv
reviewed
Oct 22, 2024
packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts
Outdated
Show resolved
Hide resolved
Haroenv
reviewed
Oct 22, 2024
packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts
Outdated
Show resolved
Hide resolved
shaejaz
requested review from
dhayab,
sarahdayan,
Haroenv and
aymeric-giraudet
Haroenv
reviewed
Oct 23, 2024
packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts
Show resolved
Hide resolved
Haroenv
approved these changes
Oct 23, 2024
This was
linked to
issues
Oct 25, 2024
JasonBerry
pushed a commit
to algolia/search-insights.js
that referenced
this pull request
Oct 31, 2024
### Context In InstantSearch, in order for user tokens to be set in time for the first query on page load, anonymous user tokens are generated and set as both the InstantSearch token and Insights token. This flow has one issue though. When `useCookies` flag needs to be changed later (such as with a cookie consent banner), a second `init` call to insights with the flag set to true does not save the existing token to a cookie. A short repro is as follows with the very latest version of InstantSearch where this behaviour was added (PR: algolia/instantsearch#6377): ```js const searchClient = algoliasearch( // ... ); window.aa('init', { appId: 'id', apiKey: 'key', }); const search = instantsearch({ indexName: 'index', searchClient, insights: true, }); search.addWidgets([ // ... ]); // This does not save the cookie atm setTimeout(() => { window.aa('init', { partial: true, useCookie: true, }); }, 5000); ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FX-2955
Summary
Double queries happen when the state of instantsearch and insights no longer matches at the time of the first query on page load.
The
search-insightsonUserTokenChangeandonAuthenticatedUserTokenChangecallbacks are not able to set theuser-tokenby the time the first search query is sent. When theuser-tokenis changed by these callbacks, the state has changed from the first query. If a new query is scheduled after this(for example with dynamic widgets), the cache misses due to the changed state and second query is made.When
useCookieis set to true, insights needs to create a anonymous token and save it as a cookie. Instantsearch would have the token updated with the callback from before, but this does not happen in time for the first query. So anonymous token generation and saving in cookies is now also handled by instantsearch.The insights queue is now being access in the middleware's
startedfunction to make sure any token set before instantsearch was started are part of the state by the time the first query is made.