feat(ts): allow custom ui state and route state in routing #4816

Haroenv · 2021-07-28T14:53:39Z

InstantSearch generic:

import algoliasearch from 'algoliasearch/lite';
import instantsearch from 'instantsearch.js/es';
import { history } from 'instantsearch.js/es/lib/routers';
import { StateMapping, UiState } from 'instantsearch.js/es/types';

type SwagIndexUiState = { swag: boolean };
type SwagUiState = { [indexName: string]: SwagIndexUiState };

// can also be written inline, no need for the type then
const stateMapping: StateMapping<SwagUiState, SwagUiState> = {
  stateToRoute(uiState) {
    return Object.keys(uiState).reduce(
      (state, indexId) => ({
        ...state,
        [indexId]: { swag: uiState[indexId].swag },
      }),
      {}
    );
  },

  routeToState(routeState = {}) {
    return Object.keys(routeState).reduce(
      (state, indexId) => ({
        ...state,
        [indexId]: routeState[indexId],
      }),
      {}
    );
  },
};

instantsearch<SwagUiState, SwagUiState>({
  indexName: '',
  searchClient: algoliasearch('', ''),
  routing: {
    router: history(),
    stateMapping,
  },
});

generic middleware

import instantsearch from 'instantsearch.js/es'
import { history } from 'instantsearch.js/es/lib/routers';
import { createRouterMiddleware } from 'instantsearch.js/es/middlewares';
import { StateMapping, UiState } from 'instantsearch.js/es/types';

type SwagIndexUiState = { swag: boolean };
type SwagUiState = { [indexName: string]: SwagIndexUiState };

const stateMapping: StateMapping<SwagUiState, SwagUiState> = {
  stateToRoute(uiState) {
    return Object.keys(uiState).reduce(
      (state, indexId) => ({
        ...state,
        [indexId]: { swag: uiState[indexId].swag },
      }),
      {}
    );
  },

  routeToState(routeState = {}) {
    return Object.keys(routeState).reduce(
      (state, indexId) => ({
        ...state,
        [indexId]: routeState[indexId],
      }),
      {}
    );
  },
};

const search = instantsearch();

search.use(
  createRouterMiddleware<SwagUiState, SwagUiState>({
    router: history(),
    stateMapping,
  })
);
search.addWidgets([instantsearch.widgets.hits({ container })]);

references

fixes DX-2298

While I couldn't find a way to make InstantSearch itself generic (this gets passed to many places, which then loses generic), using the routing middleware directly is possible like this now ```ts import instantsearch from 'instantsearch.js/es' import { history } from 'instantsearch.js/es/lib/routers'; import { createRouterMiddleware } from 'instantsearch.js/es/middlewares'; import { StateMapping, UiState } from 'instantsearch.js/es/types'; type SwagIndexUiState = { swag: boolean }; type SwagUiState = { [indexName: string]: SwagIndexUiState }; const stateMapping: StateMapping<UiState & SwagUiState, SwagUiState> = { stateToRoute(uiState) { return Object.keys(uiState).reduce( (state, indexId) => ({ ...state, [indexId]: { swag: uiState[indexId].swag }, }), {} ); }, routeToState(routeState = {}) { return Object.keys(routeState).reduce( (state, indexId) => ({ ...state, [indexId]: routeState[indexId], }), {} ); }, }; const search = instantsearch(); search.use( createRouterMiddleware<UiState & SwagUiState, SwagUiState>({ router: history(), stateMapping, }) ); search.addWidgets([instantsearch.widgets.hits({ container })]); ```

codesandbox-ci · 2021-07-28T14:55:23Z

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

Sandbox	Source
InstantSearch.js	Configuration

francoischalifour

Can we let users not have to provide UiState, but we augment it on our side?

Haroenv

is this what you expect @francoischalifour ?

src/lib/main.ts

src/index.es.ts

src/lib/__tests__/RoutingManager-test.ts

src/lib/routers/history.ts

src/lib/stateMappings/simple.ts

francoischalifour · 2021-07-30T14:08:47Z

src/middlewares/createRouterMiddleware.ts

-    router = historyRouter(),
-    stateMapping = simpleStateMapping(),
+    router = historyRouter<TRouteState>(),
+    // technically this is wrong, as without stateMapping parameter given, the routeState *must* be UiState


Can you explain a bit more?

simpleStateMapping is a StateMapping<UiState, UiState>, and createRouterMiddleware can be called with a different generic than UiState, so the types don't really match. The only thing I can think of otherwise is only conditionally allowing stateMapping to be conditional, but I couldn't find how

I tried things with branding, but still could not get anything working as soon as simpleStatemapping is actually used (as it uses UiState (const stateMapping: StateMapping<UiState, TRouteState> | StateMapping<TRouteState, TRouteState> becomes a union, and even though in that case both types are the same, typescript doesn't seem to be able to know that)

things I have tried

export type RouterProps< TUiState extends UiState = UiState, TRouteState = UiState & { ___default: true } > = TUiState extends { ___default: true } ? { router?: Router<TRouteState>; stateMapping?: never; } : { router?: Router<TRouteState>; stateMapping: StateMapping<TUiState, TRouteState>; }; export const createRouterMiddleware = < TUiState extends UiState = UiState, TRouteState = UiState & { ___default: true } >( props: RouterProps<TUiState, TRouteState> = {} as RouterProps< TUiState, TRouteState > ): InternalMiddleware<TUiState> => { const { router = historyRouter<TRouteState>(), stateMapping = simpleStateMapping<TRouteState>(), } = props;

and variations of it

Haroenv · 2021-07-30T15:44:33Z

src/middlewares/createRouterMiddleware.ts

+    // We have to cast simpleStateMapping as a StateMapping<TUiState, TRouteState>.
+    // this is needed because simpleStateMapping is StateMapping<TUiState, TUiState>.
+    // While it's only used when UiState and RouteState are the same, unfortunately
+    // TypeScript still considers them separate types.


new comment @francoischalifour

francoischalifour · 2021-08-04T11:19:51Z

src/types/middleware.ts

+export type MiddlewareDefinition<TUiState extends UiState = UiState> = {
+  onStateChange(options: { uiState: TUiState }): void;


Isn't it supposed to be this here too?

Suggested change

export type MiddlewareDefinition<TUiState extends UiState = UiState> = {

onStateChange(options: { uiState: TUiState }): void;

export type MiddlewareDefinition<TUiState = Record<string, unknown>> = {

onStateChange(options: { uiState: UiState & TUiState }): void;

I was thinking every place except the root only needs to deal with UiState and likely no extensions (not really user code), so all except the entry point expects the consumer to make the mix