feat(insights): introduce `insights` middleware (1/4) #4446

eunjae-lee · 2020-07-15T13:45:42Z

Summary

This PR introduces insights middleware.
It is to automatically setup userToken across search calls and Insights api calls.
Once this middleware is added to instantSearchInstance, when user calls

window.aa('setUserToken', userToken);
// or 
insightsClient('setUserToken', userToken);

the middleware will automatically apply the userToken to the searchParameters. Then the next search call will include this userToken.

Stacked PRs

examples

basic example: https://codesandbox.io/s/instantsearchjs-jetcz?file=/src/app.js
with search-insights umd: https://codesandbox.io/s/instantsearchjs-zn81s?file=/src/app.js
insightsClient: false: https://codesandbox.io/s/instantsearchjs-1igbb?file=/src/app.js
(working example: esm, umd)

codesandbox-ci · 2020-07-15T13:48:54Z

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 5067ec7:

Sandbox	Source
InstantSearch.js	Configuration
InstantSearch.js	PR
InstantSearch.js	PR
InstantSearch.js	PR
InstantSearch.js	PR

src/lib/InstantSearch.ts

Co-authored-by: Haroen Viaene <[email protected]>

eunjae-lee · 2020-07-16T12:46:48Z

https://codesandbox.io/s/instantsearchjs-051n8?file=/package.json

src/middleware/insights.ts

Co-authored-by: Haroen Viaene <[email protected]>

eunjae-lee · 2020-07-23T08:40:00Z

src/lib/main.ts

@@ -39,5 +40,6 @@ instantsearch.createInfiniteHitsSessionStorageCache = createInfiniteHitsSessionS
 instantsearch.highlight = helpers.highlight;
 instantsearch.snippet = helpers.snippet;
 instantsearch.insights = helpers.insights;
+instantsearch.middleware = middleware;


middleware vs middlewares

"middlewares" is not proper English.

https://en.wiktionary.org/wiki/middleware

I don't think I've seen s at the end of middleware before, though.
So you agree it's natural to have middleware although we have routers, stateMappings, connectors, widgets, right?

I think it should be middlewares, at least for the object containing all possible middlewares. Interestingly my spell-checker only accepts singular "middleware" as correct, and not "middlewares"

https://www.wordhippo.com/what-is/the-plural-of/middleware.html

While middleware is more common, it's also safe to use middlewares if we want to explicitly indicate it's a collection.

src/middleware/insights.ts

Co-authored-by: François Chalifour <[email protected]>

src/types/insights.ts

src/middleware/insights.ts

francoischalifour · 2020-08-17T08:35:48Z

src/middleware/insights.ts

+import { warning, noop } from '../lib/utils';
+
+export type InsightsProps = {
+  insightsClient: false | InsightsClient;


Accepting false for the Insights client is confusing because it can't be true. Why don't we use null as we're doing with other options?

null is a fair option, I suggested false, since that's what I did with Vue: https://github.com/algolia/vue-instantsearch/blob/master/src/mixins/widget.js#L47-L55

EDIT: it's actually true there, but null would make more sense (maybe a next major)

Yeah, null sounds good.
b0b7add

src/middleware/insights.ts

Co-authored-by: François Chalifour <[email protected]>

Haroenv

lgtm if open comments are resolved 👍

Haroenv · 2020-08-24T13:02:17Z

src/middleware/insights.ts

+  };
+};
+
+function getAppIdAndApiKey(searchClient) {


maybe add a unit test here. Since we run the tests with algoliasearch v3 and v4 separately, you should be able to cover both branches.

Other case we can add a dev dependency for "algoliasearch-v3": "npm:algoliasearch@3" and import algoliasearch from 'algoliasearc-v3'

TIL: "npm:algoliasearch@3" syntax. Nice! 7f49f25

src/types/insights.ts

Haroenv · 2020-08-24T13:04:04Z

src/middleware/index.ts

-  instantSearchInstance: InstantSearch,
-}) => MiddlewareDefinition;
-
-export { createRouter, RouterProps } from './createRouter';


not sure if anyone uses this, but this changes the API

I added them back ad0dafb
but not like before because of this
(tl;dr -> rollup failed to resolve RouterProps type from ./createRouter. It didn't happen before because this was not exported at src/lib/main.ts, but not it is.)

Co-authored-by: Haroen Viaene <[email protected]>

Haroenv

I think it's good now, assuming that nobody imported instantsearch.js/es/middleware, since that will be broken now. Since middleware is experimental, I think it's ok

francoischalifour · 2020-08-31T09:20:39Z

src/middlewares/index.ts

@@ -0,0 +1,2 @@
+export { createInsightsMiddleware } from './insights';


To be consistent with the file naming, we can rename the file createInsightsMiddleware (and perhaps createRouterMiddleware?).

183b1ca good call!

francoischalifour · 2020-08-31T09:21:56Z

src/middlewares/index.ts

@@ -0,0 +1,2 @@
+export { createInsightsMiddleware } from './insights';
+export * from './createRouter';


We exports the types from the router middleware. Any opinions on aligning the two? (either exporting the types or not exporting the types)

I think it's good to export all of them e7c7e58

src/types/index.ts

* chore: dummy commit * add sendEvent method * send event from refinementList * accept custom payload at sendEvent * delegate to onEvent or actually send event * store _insightsMiddleware in instantSearchInstance * add clickedFilters to InsightsClientMethod * rename $$type of middlewares * chore: remove ! * add sendEvent for hits * update for consistency * send view event when rendering hits * expose bindEvent to templates * handle insightsEvent from templates * allow single hit for bind/sendEventForHits * assign sendEventToInsights by the middleware * Update src/lib/insights/listener.tsx Co-authored-by: Haroen Viaene <[email protected]> * require eventName for click and conversion in hits * fix type errors * fix test cases * reuse $$type from connectors * remove $$type from middleware * Apply suggestions from code review Co-authored-by: François Chalifour <[email protected]> * fix: allow only one way for sending events * feat(insights): implement sendEvent in all the connectors (3/4) (#4463) * update infiniteHits * modify createSendEventForHits to accept index instead of helper * add sendEvent to connectAutocomplete * add sendEvent to connectGeoSearch * add sendEvent to connectHierarchicalMenu * add sendEvent to connectBreadcrumb * add sendEvent to connectMenu * add sendEvent to connectNumericMenu * extract getRefinedState in connectRange * add sendEvent to connectRange * add sendEvent to connectRatingMenu * add sendEvent to connectToggleRefinement * fix test error * fix export * fix type error * use $$type instead of hard-code * remove sendEvent from connectBreadcrumb * moved createSendEvent to the top level in the files * feat(insights): add tests (4/4) (#4464) * add tests for userToken * fix import paths * fix test cases to accept null instead of false as insightsClient * update bundlesize * fix lint errors * test sendEventToInsights * use $$type instead of hard-code * remove sendEvent from connectBreadcrumb * moved createSendEvent to the top level in the files * log insights event from storybook * add test for connectors * add tests for createSendEvent helpers * add integration tests for hits and infinite-hits widgets with bindEvent in templates * update bundlesize * Update src/connectors/hits/__tests__/connectHits-test.ts Co-authored-by: Haroen Viaene <[email protected]> * use factory function instead of globally exposed variables * clean up * update comment * use runAllMicroTasks instead of nextTick * fix: type errors * fix wrong import paths * fix: pass insightsClient to onEvent as the second parameter * update titles of describe blocks Co-authored-by: Haroen Viaene <[email protected]> Co-authored-by: Haroen Viaene <[email protected]> Co-authored-by: Haroen Viaene <[email protected]> Co-authored-by: François Chalifour <[email protected]>

Haroenv reviewed Jul 16, 2020

View reviewed changes

src/lib/InstantSearch.ts Outdated Show resolved Hide resolved

eunjae-lee added 4 commits July 16, 2020 10:45

feat: update userToken automatically

3963956

chore: fix bug where it gets token before rendering the custom widget

4331d24

chore: add access modifier

77996c9

chore: use custom widget and helper

467a648

eunjae-lee force-pushed the feat/automatic-user-token branch from 053783e to 467a648 Compare July 16, 2020 08:54

Haroenv reviewed Jul 16, 2020

View reviewed changes

src/lib/InstantSearch.ts Outdated Show resolved Hide resolved

Haroenv reviewed Jul 16, 2020

View reviewed changes

src/lib/InstantSearch.ts Outdated Show resolved Hide resolved

Update src/lib/InstantSearch.ts

ec3f7bb

Co-authored-by: Haroen Viaene <[email protected]>

eunjae-lee mentioned this pull request Jul 16, 2020

feat(connector): add refineLater #4443

Closed

chore: update token to helper directly

3ab96a2

eunjae-lee force-pushed the feat/automatic-user-token branch from 2386a91 to 3ab96a2 Compare July 16, 2020 10:13

chore: move setupUserTokenUpdater to start()

020381e

chore: move to middleware

a0cc96f

Haroenv reviewed Jul 23, 2020

View reviewed changes

src/middleware/insights.ts Outdated Show resolved Hide resolved

Eunjae Lee and others added 4 commits July 23, 2020 10:31

Update src/middleware/insights.ts

c642234

Co-authored-by: Haroen Viaene <[email protected]>

export middleware

03e52c7

fix lint error

0ae09e2

export for umd build

2417c50

eunjae-lee commented Jul 23, 2020

View reviewed changes

francoischalifour reviewed Jul 23, 2020

View reviewed changes

src/middleware/insights.ts Outdated Show resolved Hide resolved

Update src/middleware/insights.ts

090fe28

Co-authored-by: François Chalifour <[email protected]>

eunjae-lee changed the title ~~feat: update userToken automatically~~ feat: introduce insights middleware Jul 23, 2020

Merge branch 'master' into feat/automatic-user-token

c262700

Haroenv reviewed Jul 23, 2020

View reviewed changes

src/types/insights.ts Outdated Show resolved Hide resolved

update token automatically

2a49909

Haroenv reviewed Jul 23, 2020

View reviewed changes

src/middleware/insights.ts Outdated Show resolved Hide resolved

Haroenv reviewed Jul 23, 2020

View reviewed changes

src/middleware/insights.ts Outdated Show resolved Hide resolved

eunjae-lee changed the title ~~feat(insights): introduce insights middleware (1/3)~~ feat(insights): introduce insights middleware (1/n) Jul 29, 2020

eunjae-lee mentioned this pull request Jul 29, 2020

feat(insights): send events from hits and refinementList (2/4) #4456

Merged

eunjae-lee added 2 commits July 29, 2020 22:02

clean up types

000d708

fix wrong import

602037a

This was referenced Jul 31, 2020

feat(insights): implement sendEvent in all the connectors (3/4) #4463

Merged

feat(insights): add tests (4/4) #4464

Merged

francoischalifour reviewed Aug 17, 2020

View reviewed changes

Eunjae Lee and others added 3 commits August 24, 2020 11:02

Update src/middleware/insights.ts

8fd4698

Co-authored-by: François Chalifour <[email protected]>

accept null as insightsClient

b0b7add

Merge branch 'master' into feat/automatic-user-token

e2a2743

Haroenv reviewed Aug 24, 2020

View reviewed changes

Eunjae Lee and others added 5 commits August 24, 2020 15:30

Update src/types/insights.ts

21faadb

Co-authored-by: Haroen Viaene <[email protected]>

add test for getAppIdAndApiKey

7f49f25

Update src/types/insights.ts

94e394b

Co-authored-by: Haroen Viaene <[email protected]>

bring back exports

ad0dafb

rename middleware to middlewares

f34170d

eunjae-lee marked this pull request as ready for review August 24, 2020 14:16

eunjae-lee requested review from Haroenv and francoischalifour August 24, 2020 14:17

Haroenv approved these changes Aug 24, 2020

View reviewed changes

Merge branch 'master' into feat/automatic-user-token

ef95a87

francoischalifour reviewed Aug 31, 2020

View reviewed changes

eunjae-lee added 2 commits August 31, 2020 14:02

chore: rename files for better alignment

183b1ca

chore: export all types related to insights middleware

e7c7e58

eunjae-lee changed the title ~~feat(insights): introduce insights middleware (1/n)~~ feat(insights): introduce insights middleware (1/4) Sep 2, 2020

Eunjae Lee and others added 2 commits September 2, 2020 14:29

Merge branch 'master' into feat/automatic-user-token

5067ec7

eunjae-lee merged commit 9bc6359 into master Sep 2, 2020

eunjae-lee deleted the feat/automatic-user-token branch September 2, 2020 12:46

Haroenv mentioned this pull request May 21, 2021

fix(range): reset the page on refine #4760

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(insights): introduce `insights` middleware (1/4) #4446

feat(insights): introduce `insights` middleware (1/4) #4446

eunjae-lee commented Jul 15, 2020 •

edited

Loading

codesandbox-ci bot commented Jul 15, 2020 •

edited

Loading

eunjae-lee commented Jul 16, 2020

eunjae-lee Jul 23, 2020

francoischalifour Jul 23, 2020

eunjae-lee Jul 27, 2020

Haroenv Jul 30, 2020

eunjae-lee Aug 24, 2020

eunjae-lee Aug 24, 2020

francoischalifour Aug 17, 2020

Haroenv Aug 17, 2020

eunjae-lee Aug 24, 2020

Haroenv left a comment

Haroenv Aug 24, 2020

eunjae-lee Aug 24, 2020

Haroenv Aug 24, 2020

eunjae-lee Aug 24, 2020

Haroenv left a comment

francoischalifour Aug 31, 2020

eunjae-lee Aug 31, 2020

francoischalifour Aug 31, 2020

eunjae-lee Aug 31, 2020

		@@ -0,0 +1,2 @@
		export { createInsightsMiddleware } from './insights';

		@@ -0,0 +1,2 @@
		export { createInsightsMiddleware } from './insights';
		export * from './createRouter';

feat(insights): introduce insights middleware (1/4) #4446

feat(insights): introduce insights middleware (1/4) #4446

Conversation

eunjae-lee commented Jul 15, 2020 • edited Loading

Summary

codesandbox-ci bot commented Jul 15, 2020 • edited Loading

eunjae-lee commented Jul 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Haroenv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Haroenv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat(insights): introduce `insights` middleware (1/4) #4446

feat(insights): introduce `insights` middleware (1/4) #4446

eunjae-lee commented Jul 15, 2020 •

edited

Loading

codesandbox-ci bot commented Jul 15, 2020 •

edited

Loading