fix(types): improve InstantSearch types #3651
Conversation
|
Deploy preview for instantsearchjs ready! Built with commit b7b7869 |
| @@ -59,7 +64,11 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/query-rule- | |||
| container: document.createElement('div'), | |||
| }); | |||
|
|
|||
| widget.init({ helper, state: helper.state, instantSearchInstance: {} }); | |||
| widget.init!({ | |||
There was a problem hiding this comment.
Since there's no guarantee that the init or render method of the widget exists, we need to force them with ! in the tests.
There was a problem hiding this comment.
Is it not possible to solve this using conditional types to get the correct return type from queryRuleCustomData? (my fear is that the user get the same issue when calling widget.init)
There was a problem hiding this comment.
We tried some stuff like conditional types with @samouss but each solution brought other complications. @samouss has more input than me on this.
I don't think users have to manipulate widget.init except in tests as we do.
Another solution would be to enforce init, render and dispose from now on. WDYT?
There was a problem hiding this comment.
Well, I still lack context on the lib to see the impact on enforcing these methods.
But I would be very interested to see what complications you encountered with @samouss with conditional types (for my own curiosity 😉 )
There was a problem hiding this comment.
But I would be very interested to see what complications you encountered with @samouss with conditional types (for my own curiosity 😉 )
We've tried with both union types and conditional types. With the latter we're almost there but not yet. Once we put the widgets inside an array we're not able to find which kind of widget it is. It means that we only have access to all the common attributes of each kind of widget (which makes perfect sense). Here is an example. Note that I might be completely wrong about the subject though.
my fear is that the user get the same issue when calling
widget.init
This is not an issue because we call the methods for the users. They never have to call it directly.
Another solution would be to enforce init, render and dispose from now on. WDYT?
Yes that would solve the issue but I would avoid to do it right now. I would rather suggest to take this into consideration once we revamp the core of InstantSearch. Until now we can treat them as optional inside the codebase it was (almost) already the case.
francoischalifour
force-pushed
the
fix/types-instantsearch
branch
3 times, most recently
from
b3ce674
to
1404d2f
Compare
|
I updated this PR so that it reflects only changes in the source files. |
| @@ -59,7 +64,11 @@ See documentation: https://www.algolia.com/doc/api-reference/widgets/query-rule- | |||
| container: document.createElement('div'), | |||
| }); | |||
|
|
|||
| widget.init({ helper, state: helper.state, instantSearchInstance: {} }); | |||
| widget.init!({ | |||
There was a problem hiding this comment.
But I would be very interested to see what complications you encountered with @samouss with conditional types (for my own curiosity 😉 )
We've tried with both union types and conditional types. With the latter we're almost there but not yet. Once we put the widgets inside an array we're not able to find which kind of widget it is. It means that we only have access to all the common attributes of each kind of widget (which makes perfect sense). Here is an example. Note that I might be completely wrong about the subject though.
my fear is that the user get the same issue when calling
widget.init
This is not an issue because we call the methods for the users. They never have to call it directly.
Another solution would be to enforce init, render and dispose from now on. WDYT?
Yes that would solve the issue but I would avoid to do it right now. I would rather suggest to take this into consideration once we revamp the core of InstantSearch. Until now we can treat them as optional inside the codebase it was (almost) already the case.
francoischalifour
force-pushed
the
fix/types-instantsearch
branch
from
1404d2f
to
591b3de
Compare
francoischalifour
force-pushed
the
fix/types-instantsearch
branch
from
591b3de
to
c82c4f8
Compare
francoischalifour
force-pushed
the
fix/types-instantsearch
branch
from
c82c4f8
to
b7b7869
Compare
# [3.3.0](v3.2.1...v3.3.0) (2019-04-11) ### Bug Fixes * **connectQueryRules:** improve tracked refinement type ([#3648](#3648)) ([e16ad57](e16ad57)) * **currentRefinements:** don't rely on ([#3672](#3672)) ([cd64bcf](cd64bcf)) * **queryRuleCustomData:** add default template ([#3650](#3650)) ([83e9eaa](83e9eaa)) * **QueryRuleCustomData:** pass data as object to templates ([#3647](#3647)) ([b8f8b4e](b8f8b4e)) * **queryRules:** fix types and stories ([#3670](#3670)) ([ba6e2e6](ba6e2e6)) * **routing:** apply windowTitle on first load ([#3669](#3669)) ([d553502](d553502)), closes [#3667](#3667) * **routing:** support parsing URLs with up to 100 refinements ([#3671](#3671)) ([6ddcfb6](6ddcfb6)) * **RoutingManager:** avoid stale uiState ([#3630](#3630)) ([e1588aa](e1588aa)) * **types:** improve InstantSearch types ([#3651](#3651)) ([db9b91e](db9b91e)) * **ua:** Update the User-Agent to use the new format ([#3616](#3616)) ([ab84c57](ab84c57)) ### Features * **infiniteHits:** add previous button ([#3645](#3645)) ([2c9e38d](2c9e38d)) * **queryRules:** add connectQueryRules connector ([#3597](#3597)) ([924cd99](924cd99)), closes [#3599](#3599) [#3600](#3600) * **queryRules:** add context features to Query Rules ([#3617](#3617)) ([922879e](922879e)), closes [#3602](#3602) ### Reverts * feat(infiniteHits): add previous button ([214c0fc](214c0fc))
This completes the InstantSearch, Helper, Client and Widget types, and updates the code impacted.