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.

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

Typing Effect Arguments for adding listeners is difficult at the moment. #2114

Closed
moneydance opened this issue Mar 11, 2022 · 7 comments
Closed

Comments

@moneydance
Copy link

moneydance commented Mar 11, 2022

We'd like to structure our listener effects like so, colocating them in our slice file.

export const domainsSideEffectListeners: AppListener[] = [
  {
    actionCreator: removeDomain.fulfilled,
    effect: (action, listenerApi) => {
      listenerApi.extra.history.push(DOMAIN_BASE);
    },
  },
  {
    actionCreator: addDomain.fulfilled,
    effect: (action, listenerApi) => {
      const domain = action.payload;
      const [hostname] = domain.hostnames;
      listenerApi.extra.history.push(makeCategoryPath(domain.id, hostname.id));
    },
  },
];

we'd then import them and add them to our stores middleware

const listenerMiddleware = createListenerMiddleware({
  extra: { history },
});

[...domainsSideEffectListeners].forEach(listenerMiddleware.startListening);

This is proving very difficult though because getting access to the type parameters passed to startListening is impossible at the moment.

I've tried to derive the argument like so

export type Store = ReturnType<typeof initStore>;
export type RootState = ReturnType<Store['getState']>;
export type AppDispatch = Store['dispatch'];
type ListenerMiddleware = ListenerMiddlewareInstance<
  RootState,
  AppDispatch,
  { history: History.PoorMansUnknown }
>;

export type AppListener = Parameters<ListenerMiddleware['startListening']>[0];

But I'm unable to do so because Parameters won't derive overloaded arguments microsoft/TypeScript#26591. Is there anyway an explicit AddListenerOptions type be exported so I can create a well typed AppListener?

@markerikson
Copy link
Collaborator

markerikson commented Mar 11, 2022

Hmm. For reference, the type we have for that is called AddListenerOverloads, and doesn't look like it's actually publicly exported atm.

The bigger issue here is that it isn't even defined as a union of objects or anything. It's specifically a set of function overloads, with varying techniques for figuring out what the action type is depending on what fields are defined in the params object. Given that, I'm not sure it's even possible to export an object type the way you're wanting it.

I'll be honest and say I don't necessarily see a lot of a syntactic benefit to defining the listeners as an array and doing listeners.forEach().

For now, I think the best option is to use the approach shown in the "counter" example in the repo:

export function setupCounterListeners(
  startListening: AppStartListening
): Unsubscribe {
  const subscriptions = [
    startListening({
      actionCreator: counterActions.updateByPeriodically,
      effect: onUpdateByPeriodically,
    }),
    startListening({
      actionCreator: counterActions.updateByAsync,
      effect: onUpdateAsync,
    }),
  ]

  return () => {
    subscriptions.forEach((unsubscribe) => unsubscribe())
  }
}

Some similarity, but the difference is that the function exported by the slice accepts the pre-typed startListening function and calls it as many times as needed.

@moneydance
Copy link
Author

Awesome thanks I'll give this a go. Is this pattern well documented outside of the example repo? It'd be rough if people went with a different pattern in javascript that worked but was hard to use when converting to typescript.

@markerikson
Copy link
Collaborator

No, we haven't actually documented any usage patterns for how to organize listeners yet. But, this question has come up 2-3 times now, so it might be worth adding to the docs.

@moneydance
Copy link
Author

moneydance commented Mar 11, 2022

Also is there an example of typing this correctly with the extra parameter you can configure with the middleware

I did

type AppListener = ListenerMiddlewareInstance<
  RootState,
  AppDispatch,
  { history: History }
>;
export type AppStartListening = AppListener['startListening'];

which seems to be working.

@markerikson
Copy link
Collaborator

We didn't particularly anticipate people using ListenerMiddlewareInstance directly, really :) Which is why the docs show inferring the types and using TypedStartListening, etc.

@moneydance
Copy link
Author

moneydance commented Mar 11, 2022

Oh got it

export type AppStartListening = TypedStartListening<
  RootState,
  AppDispatch,
  { history: History }
>;

works well I'll go with that

@moneydance
Copy link
Author

Resolved with pattern described in examples no action needed. Thanks @markerikson for the explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants