-
Notifications
You must be signed in to change notification settings - Fork 530
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.
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
feat(useInstantSearch): expose renderState #5873
Conversation
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 a3911ae:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if at some point we want to provide render state as a generic to InstantSearch, like UI state and Route state. Currently I don't think there's a way to have a type-safe render state with custom widgets.
@@ -20,6 +22,8 @@ export function useSearchState< | |||
const indexId = searchIndex.getIndexId(); | |||
const [uiState, setLocalUiState] = useState(() => search.getUiState()); | |||
const indexUiState = uiState[indexId] as TUiState[string]; | |||
const [renderState, setRenderState] = useState(() => search.renderState); | |||
const indexRenderState = renderState[indexId] || {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why we have this {}
fallback for the index render state but it's not needed for the index UI state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getUiState uses the mounted index widgets (all) as its start value, but renderState starts with an empty object
You're right, I think it should be reworked, and instead of passing UiState it should be a list of widgets and we can infer all other things from it (uiState, renderState), but not easy to do that backwards compatible / correctly |
Summary
The renderState is useful for interacting with a different widget that you know / assume is also mounted. This is good for example for refining within a hit component without adding a second refinement widget (which could have side effects if hits is conditional, despite that impact being made smaller through #5123).
Can also be useful for a reusable widget that allows the user to pass their own options to e.g. refinementList, but still expects it to be mounted by the user.
An example use case:
Result
RFC-71 is implemented more completely