feat(core): expose metadata of widgets #4604

Haroenv · 2020-12-09T09:30:28Z

this adds a new middleware (enabled by default) which calls getWidgetRenderState on all widgets to get the widgetParams

codesandbox-ci · 2020-12-09T09:32:33Z

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 fa6a645:

Sandbox	Source
InstantSearch.js	Configuration

src/middlewares/createTelemetryMiddleware.ts

src/widgets/places/places.ts

src/middlewares/createTelemetryMiddleware.ts

src/types/widget.ts

src/lib/InstantSearch.ts

when the user agent of the crawler is detected a meta tag is added to the document head with the widget parameters for the added widgets. Co-authored-by: François Chalifour <[email protected]> Co-authored-by: Eunjae Lee <[email protected]> Co-authored-by: Yannick Croissant <[email protected]> Co-authored-by: Clément Vannicatte <[email protected]>

src/middlewares/createTelemetryMiddleware.ts

i noticed this was the only place where the unintuitive $0 is used after #4604, so i suggest this change as well

src/middlewares/createTelemetryMiddleware.ts

src/widgets/panel/panel.tsx

src/middlewares/createTelemetryMiddleware.ts

eunjae-lee

It looks good to me. It'd be good to have a snapshot test on the telemetry payload, so that we can have a quick glance on how it looks like, but not a blocker to merge the PR.

yannickcr · 2021-01-04T16:46:21Z

src/middlewares/createTelemetryMiddleware.ts

+    let widgetParams = {};
+
+    if (widget.getWidgetRenderState) {
+      const renderState = widget.getWidgetRenderState(initOptions);


The problem is by using widgetParams from the renderState you do not get the parameters used by the user but the ones passed to the connector, that can be different depending of what was done in the widget (some are not passed to the connector, others can added or transformed). So we do not really get a view of what parameters was used by the user, defeating the main purpose of the project 😢

I actually had a look, and while we are missing the parameters that are not sent to the connector (visual-only ones), we still retrieve all the values without their defaults, since those are only assigned on the destructuring on the connector. I think that's a fair tradeoff that doesn't require rewriting all widgets. I will however after this do a pass on all widgets, and make sure they don't set any defaults that are also set in the connectors

I think in a second pass, we actually could send the visual-only parameters to the connector as well, since it's impossible for the parameters to clash significantly

I think this is a mandatory step, in the current PR we are missing a lot of information (see https://github.com/algolia/instantsearch.js/blob/2431b966e771d8d9386d919eb469c79b27b5b24a/src/widgets/pagination/pagination.js for example, only totalPages and padding are sent to the connector, we are missing templates, showFirst, showLast, etc.)

Also in most widget the parameters are send after destructuring, so with their default values.

so with their default values

I've checked, and that's not really true. They're destructured, but the default value is applied in the connector

src/middlewares/createTelemetryMiddleware.ts

src/types/widget.ts

francoischalifour · 2021-01-11T10:05:25Z

src/types/widget.ts

+  /**
+   * private marker on widgets to differentiate a widget from a connector
+   */
+  $$official?: boolean;


Why can't we base this on the ais namespace? ($$type.startsWith('ais.')).

It'd be nice not having to extend our APIs for this.

the goal is to differentiate between a widget and a connector. Do you see another way?

going with $widgetType in a separate PR

francoischalifour

We don't test that the Telemetry middleware is added to the InstantSearch instance when the crawler agent is present.

src/middlewares/__tests__/createTelemetryMiddleware.ts

src/middlewares/createTelemetryMiddleware.ts

…tion

src/middlewares/createMetadataMiddleware.ts

this prevents it from being classed as a response

Haroenv commented Dec 18, 2020

View reviewed changes

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

src/widgets/places/places.ts Outdated Show resolved Hide resolved

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

Haroenv changed the title ~~Poc/telemetry4~~ feat(core): add metadata of widgets to head Dec 18, 2020

Haroenv changed the title ~~feat(core): add metadata of widgets to head~~ feat(core): add metadata of widgets to head on crawler Dec 18, 2020

Haroenv marked this pull request as ready for review December 18, 2020 10:10

Haroenv requested a review from a team December 18, 2020 10:25

ghost requested review from eunjae-lee and shortcuts and removed request for a team December 18, 2020 10:25

eunjae-lee reviewed Dec 18, 2020

View reviewed changes

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

eunjae-lee reviewed Dec 18, 2020

View reviewed changes

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

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

Haroenv force-pushed the poc/telemetry4 branch from 6cef37d to 45fc64c Compare December 18, 2020 14:46

Haroenv commented Dec 18, 2020

View reviewed changes

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

Haroenv force-pushed the poc/telemetry4 branch from 45fc64c to a18daff Compare December 18, 2020 15:05

Haroenv added 4 commits December 22, 2020 09:40

Merge branch 'master' into poc/telemetry4

15725fe

use newest helper

2c48e09

fix eslint

e27c61c

simplify

b5191f5

Haroenv added a commit that referenced this pull request Dec 22, 2020

chore(utils): simplify jsdoc

91f07fa

i noticed this was the only place where the unintuitive $0 is used after #4604, so i suggest this change as well

eunjae-lee reviewed Dec 23, 2020

View reviewed changes

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

src/widgets/panel/panel.tsx Outdated Show resolved Hide resolved

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

Haroenv added 3 commits December 23, 2020 15:32

remove unused $$params

efeac96

extract payload recursively & safer

74d3c13

Merge branch 'master' into poc/telemetry4

ce45dfe

Haroenv requested a review from eunjae-lee December 23, 2020 14:50

eunjae-lee reviewed Dec 23, 2020

View reviewed changes

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

eunjae-lee approved these changes Dec 23, 2020

View reviewed changes

Haroenv added 3 commits December 24, 2020 11:22

fix(index): add official widget disclaimer

51090f8

chore: consistent import

d8ea0e6

test(telemetry): add unitary test

2431b96

shortcuts approved these changes Jan 4, 2021

View reviewed changes

yannickcr reviewed Jan 4, 2021

View reviewed changes

Haroenv changed the title ~~feat(core): add metadata of widgets to head on crawler~~ feat(core): expose metadata of widgets Jan 5, 2021

rename officialWidget to official

faac1ac

francoischalifour reviewed Jan 11, 2021

View reviewed changes

Haroenv added 5 commits January 12, 2021 18:10

Merge branch 'master' into poc/telemetry4

bf94298

remove $$official (for $$widgetType in separate PR)

efb8666

move DOM modification to subscribe callback

0089efc

move telemetry enabled

7c4d8ee

fix capitalisation

40e91b0

Haroenv requested a review from francoischalifour January 12, 2021 17:11

francoischalifour requested changes Jan 13, 2021

View reviewed changes

src/middlewares/__tests__/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

src/middlewares/createTelemetryMiddleware.ts Outdated Show resolved Hide resolved

Haroenv added 3 commits January 13, 2021 15:31

no fallback for type (crawler does it)

e75fbce

rename to metadata

caca78b

test(InstantSearch): assert that middleware gets added in right condi…

bee6937

…tion

Haroenv requested a review from francoischalifour January 13, 2021 15:04

francoischalifour reviewed Jan 14, 2021

View reviewed changes

src/middlewares/createMetadataMiddleware.ts Outdated Show resolved Hide resolved

src/middlewares/createMetadataMiddleware.ts Show resolved Hide resolved

create meta tag on middleware creation instead of "creator"

ca1d86d

Haroenv requested a review from francoischalifour January 14, 2021 13:59

Haroenv added 3 commits January 14, 2021 15:03

add comment

897ccfb

Merge branch 'master' into poc/telemetry4

ea332d8

chore: use empty scopedResults

fa6a645

this prevents it from being classed as a response

Haroenv merged commit 1fcf716 into master Jan 19, 2021

Haroenv deleted the poc/telemetry4 branch January 19, 2021 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): expose metadata of widgets #4604

feat(core): expose metadata of widgets #4604

Haroenv commented Dec 9, 2020 •

edited

Loading

codesandbox-ci bot commented Dec 9, 2020 •

edited

Loading

eunjae-lee left a comment

yannickcr Jan 4, 2021

Haroenv Jan 5, 2021

Haroenv Jan 5, 2021

yannickcr Jan 6, 2021

Haroenv Jan 11, 2021

francoischalifour Jan 11, 2021

Haroenv Jan 11, 2021

Haroenv Jan 12, 2021

Haroenv Jan 13, 2021

francoischalifour left a comment

feat(core): expose metadata of widgets #4604

feat(core): expose metadata of widgets #4604

Conversation

Haroenv commented Dec 9, 2020 • edited Loading

codesandbox-ci bot commented Dec 9, 2020 • edited Loading

eunjae-lee 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

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

francoischalifour left a comment

Choose a reason for hiding this comment

Haroenv commented Dec 9, 2020 •

edited

Loading

codesandbox-ci bot commented Dec 9, 2020 •

edited

Loading