refactor(ts): migrate connectors to new Widget type #4735

Haroenv · 2021-04-15T13:24:35Z

Summary

migrates all the connectors to the new type from #4734

Result

fixes some smaller things in the tests, plus implementation of refinementList (removed dead code)

connectToggleRefinement: nest getRenderState per attribute
connectToggleRefinement: remove state state from getWidgetRenderState
connectRefinementList: remove dead code in getWidgetRenderState
connectRelevantSort: remove "relevantSort" nesting, since there's only one property

These are technically breaking changes, but small enough to just be listed in the changeling IMO

Haroenv · 2021-04-15T13:44:56Z

src/connectors/answers/connectAnswers.ts

+  $$type: 'ais.answers';
+  renderState: AnswersRenderState;
+  indexRenderState: {
+    answers: WidgetRenderState<AnswersRenderState, AnswersConnectorParams>;


Suggested change

answers: WidgetRenderState<AnswersRenderState, AnswersConnectorParams>;

answers: AnswersRenderState;

I wish it could just be this, and then augmented with the parameters when you use it (eg. in Widget['getRenderState']), but I can't get it to work, so for now, the index render state only types the connector params, not the connector & widget params. I think this is a reasonable tradeoff for now, considering the use cases for reading the widget params from index render state are likely slim (and can use an as any)

Looks good to me. We could improve this part later if we find a solution.

codesandbox-ci · 2021-04-15T14:34:39Z

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 049d4b8:

Sandbox	Source
InstantSearch.js	Configuration

Haroenv · 2021-04-19T15:44:52Z

src/connectors/relevant-sort/__tests__/connectRelevantSort-test.ts

-        relevantSort: { relevancyStrictness: undefined },
+        relevantSort: 25,


I don't understand why it was 25 and not undefined before, this is a fix

When the helper state doesn't include appliedRelevancyStrictness at all, then the widget shouldn't, either. I guess that's what this test is about.

but if the helper state doesn't have relevancy strictness, but the ui state (route) does have it, doesn't that apply the strictness in getWidgetSearchParameters?

yeah it sounds right.

Haroenv · 2021-04-19T15:45:51Z

src/connectors/relevant-sort/connectRelevantSort.ts

-          relevantSort: {
-            ...uiState.relevantSort,
-            relevancyStrictness: searchParameters.relevancyStrictness,
-          },
+          relevantSort:
+            searchParameters.relevancyStrictness || uiState.relevantSort,


uiState change is technically a breaking change, but small enough to just mention in changelog

|| uiState.relevantSort

Does this mean we can now set the relevantSort from the frontend?

Haroenv · 2021-04-19T15:46:06Z

src/connectors/toggle-refinement/connectToggleRefinement.js

-          toggleRefinement: this.getWidgetRenderState(renderOptions),
+          toggleRefinement: {
+            ...renderState.toggleRefinement,
+            [attribute]: this.getWidgetRenderState(renderOptions),
+          },


uiState change is technically a breaking change, but small enough to just mention in changelog

Haroenv · 2021-04-20T08:02:22Z

src/connectors/refinement-list/connectRefinementList.ts

@@ -385,25 +384,13 @@ const connectRefinementList: RefinementListConnector = function connectRefinemen
        }

        if (results) {
-          if (!isFromSearch) {


this code was unreachable, isFromSearch is never passed to getWidgetRenderState

Haroenv · 2021-04-20T08:03:48Z

src/types/widget.ts

-  : Partial<RequiredWidgetType<TWidgetDescription>>;
+  : {
+      /**
+       * Identifier for widgets.
+       */
+      $$widgetType?: string;
+    };


previous implementation sometimes caused infinite loops in the type resolution, so it's written explicitly here

eunjae-lee

Thanks for the work!

src/connectors/refinement-list/connectRefinementList.ts

eunjae-lee · 2021-04-21T13:29:59Z

src/connectors/relevant-sort/__tests__/connectRelevantSort-test.ts

-        relevantSort: { relevancyStrictness: undefined },
+        relevantSort: 25,


When the helper state doesn't include appliedRelevancyStrictness at all, then the widget shouldn't, either. I guess that's what this test is about.

eunjae-lee · 2021-04-21T13:35:41Z

src/connectors/answers/connectAnswers.ts

+  $$type: 'ais.answers';
+  renderState: AnswersRenderState;
+  indexRenderState: {
+    answers: WidgetRenderState<AnswersRenderState, AnswersConnectorParams>;


Looks good to me. We could improve this part later if we find a solution.

eunjae-lee

Looks good to me!

src/connectors/refinement-list/connectRefinementList.ts

eunjae-lee · 2021-04-22T09:19:10Z

src/connectors/relevant-sort/__tests__/connectRelevantSort-test.ts

-        relevantSort: { relevancyStrictness: undefined },
+        relevantSort: 25,


yeah it sounds right.

shortcuts · 2021-05-03T11:19:47Z

src/connectors/relevant-sort/connectRelevantSort.ts

-          relevantSort: {
-            ...uiState.relevantSort,
-            relevancyStrictness: searchParameters.relevancyStrictness,
-          },
+          relevantSort:
+            searchParameters.relevancyStrictness || uiState.relevantSort,


|| uiState.relevantSort

Does this mean we can now set the relevantSort from the frontend?

Haroenv mentioned this pull request Apr 15, 2021

refactor(ts): migrate widgets #4737

Merged

Haroenv commented Apr 15, 2021

View reviewed changes

Haroenv force-pushed the chore/types-reorg-5 branch from 52671f2 to 3f3da50 Compare April 15, 2021 14:31

Haroenv commented Apr 19, 2021

View reviewed changes

Haroenv requested review from a team, eunjae-lee and tkrugg and removed request for a team April 19, 2021 15:46

Haroenv commented Apr 20, 2021

View reviewed changes

eunjae-lee reviewed Apr 21, 2021

View reviewed changes

eunjae-lee approved these changes Apr 22, 2021

View reviewed changes

Haroenv requested a review from shortcuts May 3, 2021 07:57

shortcuts approved these changes May 3, 2021

View reviewed changes

Haroenv force-pushed the chore/types-reorg-4 branch from 01ece8c to 53d4812 Compare May 3, 2021 12:45

Base automatically changed from chore/types-reorg-4 to master May 3, 2021 12:45

Haroenv added 4 commits May 3, 2021 14:46

refactor(ts): migrate connectors to new Widget type

66fe9ec

refactor(ts): undo wrong change

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

f905b36

update

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

160df19

update tests

049d4b8

Haroenv force-pushed the chore/types-reorg-5 branch from cd4e053 to 049d4b8 Compare May 3, 2021 12:46

Haroenv merged commit f742083 into master May 3, 2021

Haroenv deleted the chore/types-reorg-5 branch May 3, 2021 12:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ts): migrate connectors to new Widget type #4735

refactor(ts): migrate connectors to new Widget type #4735

Haroenv commented Apr 15, 2021 •

edited

Loading

Haroenv Apr 15, 2021

eunjae-lee Apr 21, 2021

codesandbox-ci bot commented Apr 15, 2021 •

edited

Loading

Haroenv Apr 19, 2021 •

edited

Loading

eunjae-lee Apr 21, 2021

Haroenv Apr 21, 2021

eunjae-lee Apr 22, 2021

Haroenv Apr 19, 2021

shortcuts May 3, 2021

Haroenv Apr 19, 2021

Haroenv Apr 20, 2021

Haroenv Apr 20, 2021 •

edited

Loading

eunjae-lee left a comment

eunjae-lee Apr 21, 2021

eunjae-lee Apr 21, 2021

eunjae-lee left a comment

eunjae-lee Apr 22, 2021

shortcuts May 3, 2021

	answers: WidgetRenderState<AnswersRenderState, AnswersConnectorParams>;
	answers: AnswersRenderState;

		relevantSort: { relevancyStrictness: undefined },
		relevantSort: 25,

refactor(ts): migrate connectors to new Widget type #4735

refactor(ts): migrate connectors to new Widget type #4735

Conversation

Haroenv commented Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codesandbox-ci bot commented Apr 15, 2021 • edited Loading

Haroenv Apr 19, 2021 • edited Loading

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 Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

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

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

Haroenv commented Apr 15, 2021 •

edited

Loading

codesandbox-ci bot commented Apr 15, 2021 •

edited

Loading

Haroenv Apr 19, 2021 •

edited

Loading

Haroenv Apr 20, 2021 •

edited

Loading