Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(widgets): add $$type to widgets definition #3960

Merged
merged 1 commit into from
Jul 18, 2019
Merged

feat(widgets): add $$type to widgets definition #3960

merged 1 commit into from
Jul 18, 2019

Conversation

francoischalifour
Copy link
Member

Adding a type to the index widget is necessary to filter it out from the list of the widgets. This is needed to implement the "reset page" behavior to get all the index-type children and reset their own page.

I took this opportunity to add a type to all widgets for debugging purpose. The widgets sharing the same connector have the same type. This is fine for now. We can provide an option to override it from the widget parameters later if needed.

The $$type property is not required in TypeScript because once we expose the widget interface, users might not need to mark them.

@francoischalifour
feat(widgets): add $$type to widgets definition
6bbce41 
Adding a type to the `index` widget is necessary to filter it out from the list of the widgets. This is needed to implement the "reset page" behavior to get all the `index`-type children and reset their own page.

I took this opportunity to add a type to all widgets for debugging purpose.

The `$$type` property is not required in TypeScript because once we expose the widget interface, users might not need to mark them.
@francoischalifour francoischalifour requested a review from a team July 17, 2019 13:26
@ghost ghost requested review from Haroenv and tkrugg and removed request for a team July 17, 2019 13:31
@algobot
Copy link
Contributor

algobot commented Jul 17, 2019

Deploy preview for instantsearchjs ready!

Built with commit 6bbce41

https://deploy-preview-3960--instantsearchjs.netlify.com

Haroenv
Haroenv reviewed Jul 17, 2019
View reviewed changes
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test which loops over all widgets and checks if it has a $$type? I love this change by the way, it will make debugging widgets easier, I often log them out and try to find out the name of the widget, but it doesn't work once minified.

@francoischalifour
Copy link
Member Author

@Haroenv I added an assertion ensuring that widgets are "somehow widgets" in each file: https://github.com/algolia/instantsearch.js/blob/6bbce4111b31a04fecd15e7bf4b21d649258cc73/src/connectors/hierarchical-menu/__tests__/connectHierarchicalMenu-test.js#L56-L66

The goal would be to make this assertion stricter with time by removing the expect.objectContaining() because we sometimes return more objects in this scope (some are used for testing purpose).

I'm not a fan of generated tests. We only do that to loop over all exported widgets to check that they are functions. Do you think these should also add an assertion for $$type?

https://github.com/algolia/instantsearch.js/blob/6bbce4111b31a04fecd15e7bf4b21d649258cc73/src/lib/__tests__/main-test.js#L10-L14

These checks seem to be here to compensate the lack of integration tests.

Another solution would be to make $$type required and to not export the internal widget type to users.

@Haroenv
Copy link
Contributor

Haroenv commented Jul 17, 2019

Yes, but that would still be only catch those written in TS which actually correctly implement the type, which is already most ideally. I thought we could quickly test something like this, but I understand your POV

samouss
samouss approved these changes Jul 18, 2019
View reviewed changes
Copy link
Contributor

@samouss samouss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those tests are enough IMO.

@francoischalifour francoischalifour merged commit 6153a72 into next Jul 18, 2019
@francoischalifour francoischalifour deleted the feat/widgets-type branch July 18, 2019 11:19
samouss pushed a commit that referenced this pull request Sep 17, 2019
@francoischalifour @samouss
feat(widgets): add $$type to widgets definition (#3960)
22138b8 
Adding a type to the `index` widget is necessary to filter it out from the list of the widgets. This is needed to implement the "reset page" behavior to get all the `index`-type children and reset their own page.

I took this opportunity to add a type to all widgets for debugging purpose.

The `$$type` property is not required in TypeScript because once we expose the widget interface, users might not need to mark them.
samouss pushed a commit that referenced this pull request Sep 17, 2019
@francoischalifour @samouss
feat(widgets): add $$type to widgets definition (#3960)
0293d7f 
Adding a type to the `index` widget is necessary to filter it out from the list of the widgets. This is needed to implement the "reset page" behavior to get all the `index`-type children and reset their own page.

I took this opportunity to add a type to all widgets for debugging purpose.

The `$$type` property is not required in TypeScript because once we expose the widget interface, users might not need to mark them.
samouss pushed a commit that referenced this pull request Sep 17, 2019
@francoischalifour @samouss
feat(widgets): add $$type to widgets definition (#3960)
f2ddaf0 
Adding a type to the `index` widget is necessary to filter it out from the list of the widgets. This is needed to implement the "reset page" behavior to get all the `index`-type children and reset their own page.

I took this opportunity to add a type to all widgets for debugging purpose.

The `$$type` property is not required in TypeScript because once we expose the widget interface, users might not need to mark them.
Haroenv pushed a commit that referenced this pull request Oct 23, 2019
@francoischalifour @Haroenv
feat(widgets): add $$type to widgets definition (#3960)
344d1b7 
Adding a type to the `index` widget is necessary to filter it out from the list of the widgets. This is needed to implement the "reset page" behavior to get all the `index`-type children and reset their own page.

I took this opportunity to add a type to all widgets for debugging purpose.

The `$$type` property is not required in TypeScript because once we expose the widget interface, users might not need to mark them.
Haroenv added a commit that referenced this pull request Oct 23, 2019
@Haroenv
v4.0.0
92b07d1 
* **configure:** merge with the previous parameters ([#4085](#4085)) ([a215d0c](a215d0c))
* **configure:** update lifecycle state ([#3994](#3994)) ([3d8d967](3d8d967))
* **connectInfiniteHits:** fix page state when adding or removing widgets ([#4104](#4104)) ([1077340](1077340))
* **connectInfiniteHits:** fix state when navigating or adding/removing widgets ([#4123](#4123)) ([9cbd24a](9cbd24a))
* **createURL:** support multi-index ([#4082](#4082)) ([179a6e5](179a6e5))
* **defer:** recover from error ([#3933](#3933)) ([f22b9e2](f22b9e2))
* **helper:** expose .lastResults to .helper ([#4170](#4170)) ([236eb7b](236eb7b))
* **history:** avoid empty query string ([#4130](#4130)) ([18fee7c](18fee7c))
* **hits:** update lifecycle state ([#3977](#3977)) ([6e55ba6](6e55ba6))
* **hitsPerPage:** avoid sync default value ([#4086](#4086)) ([3f8b958](3f8b958))
* **hitsPerPage:** update lifecycle state ([#3978](#3978)) ([d21d620](d21d620))
* **index:** ensure that we always use the index set by widgets ([#4125](#4125)) ([952dc70](952dc70)), closes [/github.com/algolia/algoliasearch-helper-js/blob/5a0352aa233c5ea932df6b054a16989c8d302404/src/algoliasearch.helper.js#L124](https://github.com//github.com/algolia/algoliasearch-helper-js/blob/5a0352aa233c5ea932df6b054a16989c8d302404/src/algoliasearch.helper.js/issues/L124)
* **index:** prevent render without results ([#3932](#3932)) ([1b9b5f4](1b9b5f4))
* **index:** subscribe to state change only after init for uiState ([#4003](#4003)) ([9490ca9](9490ca9))
* **index:** support custom UI params in UI state warning ([#4165](#4165)) ([80d32fc](80d32fc))
* **index:** warn for inconsistent UI state in development mode ([#4140](#4140)) ([7e277dc](7e277dc))
* **infiniteHits:** update lifecycle state ([#3983](#3983)) ([4b8bee5](4b8bee5))
* **instantsearch:** return instance in widgets methods ([#4143](#4143)) ([77ffb93](77ffb93))
* **InstantSearch:** cancel scheduled operations ([#3930](#3930)) ([3aafbad](3aafbad))
* **InstantSearch:** fix initialUIState when refinements are already present in the route ([#4103](#4103)) ([079db57](079db57))
* **InstantSearch:** remove useless walk/duplicate request ([#4127](#4127)) ([70163a8](70163a8))
* **menu:** apply & remove refinement ([#4027](#4027)) ([85de2cf](85de2cf))
* **menu:** prevent error on stale search ([#3934](#3934)) ([5f9e138](5f9e138))
* **numericMenu:** take array into account for empty state ([#4084](#4084)) ([2c05a01](2c05a01))
* **pagination:** update lifecycle state ([#3979](#3979)) ([2b08344](2b08344))
* **pagination:** update no refinement behavior ([#4124](#4124)) ([8d222ad](8d222ad))
* **range:** clear widget state on empty refinements ([#4157](#4157)) ([23cd112](23cd112))
* **ratingMenu:** update lifecycle state ([#3987](#3987)) ([ffadf64](ffadf64))
* **RefinementList:** remove root css class on sublists ([#4117](#4117)) ([ceddd42](ceddd42)), closes [/github.com/algolia/instantsearch.js/blob/v2/src/decorators/headerFooter.js#L22](https://github.com//github.com/algolia/instantsearch.js/blob/v2/src/decorators/headerFooter.js/issues/L22)
* **searchBox:** update lifecycle state ([#3981](#3981)) ([0ea4950](0ea4950))
* **sortBy:** ensure a return value for getWidgetSearchParameters ([#4126](#4126)) ([569d573](569d573))
* **sortBy:** read initial index name from parent index ([#4079](#4079)) ([fe23c55](fe23c55))
* display warnings only in development ([#4150](#4150)) ([44f69a0](44f69a0))
* remove useless types  ([#3958](#3958)) ([ddebf53](ddebf53))
* **stories:** hide Places ([#4152](#4152)) ([7ff843f](7ff843f))
* **toggleRefinement:** update lifecycle state ([#3993](#3993)) ([f1beff6](f1beff6))
* **voiceSearch:** update lifecycle state ([#3982](#3982)) ([798e3c1](798e3c1))
* **warnings:** remove v3 warnings ([#4134](#4134)) ([7eb6810](7eb6810))

* **autocomplete:** leverage scoped results ([#3975](#3975)) ([8f05968](8f05968))
* **autocomplete:** participate in routing ([#4029](#4029)) ([a9ca0c5](a9ca0c5))
* **autocomplete:** provide indexId ([#4142](#4142)) ([b641e23](b641e23))
* **clearRefinements:** support multiple indices ([#4036](#4036)) ([3611b11](3611b11))
* **connectAutocomplete:** add default value on getConfiguration ([#3836](#3836)) ([724b83f](724b83f))
* **connectAutocomplete:** clear the state on dispose ([#3815](#3815)) ([8ae87d8](8ae87d8))
* **connectHierarchicalMenu:** update getWidgetSearchParameters ([#4053](#4053)) ([c99f822](c99f822))
* **connectHits:** clear the state on dispose ([#3816](#3816)) ([c4de730](c4de730))
* **connectHits:** implement getWidgetSearchParameters ([#4001](#4001)) ([c77cf66](c77cf66))
* **connectHitsPerPage:** clear the state on dispose ([#3818](#3818)) ([d7a5c89](d7a5c89))
* **connectInfiniteHits:** add default value on getConfiguration ([#3837](#3837)) ([8c65249](8c65249))
* **connectInfiniteHits:** clear the state on dispose ([#3819](#3819)) ([60ce151](60ce151))
* **connectMenu:** update getWidgetSearchParameters ([#4054](#4054)) ([7d001e7](7d001e7))
* **connectNumericMenu:** update state lifecycle  ([#4013](#4013)) ([2620c90](2620c90))
* **connectPagination:** add default value on getConfiguration ([#3838](#3838)) ([aa4602c](aa4602c))
* **connectPagination:** clear the state on dispose ([#3821](#3821)) ([5b8ef49](5b8ef49))
* **connectPagination:** update getWidgetSearchParameters ([#4004](#4004)) ([eed7e77](eed7e77))
* **connectRange:** default `precision` to 0 ([#3953](#3953)) ([632e06b](632e06b))
* **connectRatingMenu:** update getWidgetSearchParameters  ([#4008](#4008)) ([d3c96bf](d3c96bf))
* **connectRefinementList:** update getWidgetSearchParameters  ([#4010](#4010)) ([ddc8fc4](ddc8fc4))
* **connectSearchBox:** clear the state on dispose ([#3822](#3822)) ([940522c](940522c))
* **connectSearchBox:** mount with a default query ([#3840](#3840)) ([c3a7d69](c3a7d69))
* **connectSearchBox:** update getWidgetSearchParameters ([#4002](#4002)) ([5c6fcd8](5c6fcd8))
* **connectVoiceSearch:** add default value on getConfiguration ([#3841](#3841)) ([fb70363](fb70363))
* **connectVoiceSearch:** clear the state on dispose ([#3823](#3823)) ([705b3e6](705b3e6))
* **connectVoiceSearch:** update getWidgetSearchParameters ([#4055](#4055)) ([b8c669f](b8c669f))
* **core:** deprecate addWidget & removeWidget ([#4131](#4131)) ([e5dafef](e5dafef))
* **currentRefinements:** support multiple indices ([#4012](#4012)) ([e997728](e997728))
* **defer:** implement cancellable callback ([#3916](#3916)) ([43a0bf8](43a0bf8))
* **federated:** keep a consistent state in the RefinementList life cycle ([#3976](#3976)) ([31d0fd6](31d0fd6))
* **hitsPerPage:** support new routing system ([#4038](#4038)) ([02502cb](02502cb)), closes [#4069](#4069)
* **index:** accept indexId ([#4070](#4070)) ([b74f8e3](b74f8e3))
* **index:** add mergeSearchParameters function ([#3917](#3917)) ([c0fe7bb](c0fe7bb))
* **index:** add widget ([dbbda0f](dbbda0f)), closes [#3892](#3892) [#3893](#3893) [#3914](#3914)
* **index:** compute local uiState ([#3997](#3997)) ([997c0f4](997c0f4))
* **index:** merge `ruleContexts` search parameter ([#3944](#3944)) ([e94752d](e94752d))
* **index:** provide scoped results to render hook ([#3964](#3964)) ([37c6aad](37c6aad))
* **index:** replicate searchFunction hack ([#4078](#4078)) ([1d2a816](1d2a816)), closes [/github.com/algolia/instantsearch.js/blob/509513c0feafaad522f6f18d87a441559f4aa050/src/lib/RoutingManager.ts#L113-L130](https://github.com//github.com/algolia/instantsearch.js/blob/509513c0feafaad522f6f18d87a441559f4aa050/src/lib/RoutingManager.ts/issues/L113-L130)
* **index:** reset page of child indexes ([#3962](#3962)) ([131b1ce](131b1ce))
* **index:** resolve parent SearchParameters ([#3937](#3937)) ([2611da5](2611da5))
* **index:** use uiState driven SearchParameters ([#4059](#4059)) ([b12bb9f](b12bb9f))
* **infiniteHits:** support new routing system ([#4040](#4040)) ([49315cf](49315cf))
* **instantsearch:** add onStateChange method ([#4080](#4080)) ([9f68da5](9f68da5))
* **InstantSearch:** switch to DerivedHelper only ([#3885](#3885)) ([d6fc317](d6fc317))
* **places:** add Places widget ([#4167](#4167)) ([1d754d1](1d754d1))
* drop support of searchParameters for initialUiState ([#4081](#4081)) ([571efeb](571efeb))
* **range:** support new routing system ([#4039](#4039)) ([8cba05a](8cba05a))
* **routing:** add a "single index" compatibility mode ([#4087](#4087)) ([842eb0f](842eb0f))
* **RoutingManager:** update state on route update ([#4100](#4100)) ([88f2615](88f2615))
* **toggleRefinement:** support new routing system ([#4037](#4037)) ([6a9d99f](6a9d99f))
* **types:** DerivedHelper ([#3887](#3887)) ([0f38b4a](0f38b4a))
* **types:** rename RenderOptions -> RendererOptions ([#3867](#3867)) ([05c6f72](05c6f72))
* **utils:** implement defer ([#3882](#3882)) ([8af470e](8af470e))
* **voice:** add additional query parameters ([#3738](#3738)) ([c555255](c555255))
* drop suppot for onHistoryChange ([#3941](#3941)) ([697f609](697f609))
* introduce initialUiState option ([#4074](#4074)) ([de00707](de00707))
* update UiState definition ([#4075](#4075)) ([9e7d3d8](9e7d3d8))
* **widgets:** add `$$type` to widgets definition ([#3960](#3960)) ([344d1b7](344d1b7))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samouss samouss samouss approved these changes

@Haroenv Haroenv Haroenv approved these changes

@tkrugg tkrugg Awaiting requested review from tkrugg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@francoischalifour @algobot @Haroenv @samouss