reduxjs / redux-devtools

DevTools for Redux with hot reloading, action replay, and customizable UI
http://youtube.com/watch?v=xsSnOQynTHs
MIT License
14.05k stars 1.17k forks source link

TypeScript error when using `stateSanitizer` or `actionSanitizer` with custom state/action types #1534

Open OliverJAsh opened 1 year ago

OliverJAsh commented 1 year ago

Reduced test case:

import { composeWithDevTools } from "@redux-devtools/extension";

type MyState = {
  foo: string;
};

composeWithDevTools({
  // ❌ Error
  stateSanitizer: (state: MyState) => state,
});

type MyAction = {
  type: "foo";
};

composeWithDevTools({
  // ❌ Error
  actionSanitizer: (action: MyAction) => action,
});

I raised a PR to fix this in the old repository: https://github.com/zalmoxisus/redux-devtools-extension/pull/716. If this looks good, I'd be happy raise another PR.

Methuselah96 commented 12 months ago

Thanks for the issue!

Seems worth fixing, but I'm not sure making EnhancerOptions, composeWithDevTools, and devToolsEnhancer generic would be the right solution since there's no way to guarantee that a certain enhancer receives a certain state/action at the type or runtime level.

I think the right fix would be to make the state types unknown and the action types Action<string>. The user can then cast those if they know the enhancer will only receive their state/action types.

OliverJAsh commented 12 months ago

there's no way to guarantee that a certain enhancer receives a certain state/action at the type

Could we change the Redux types to pass the state and action generics through to StoreEnhancer e.g. from StoreCreator? This would be a bigger change of course, but I think that would make most sense when type safety is the goal.

Methuselah96 commented 12 months ago

Maybe?

  1. Composition of enhancers might be tricky (i.e., making sure that the any augmented state/dispatch is being passed correctly through the chain of enhancers).
  2. We would need to be able to express both enhancers that work with any state/action and enhancers that only work with a predefined state/action. (This may not be hard, just making sure it's considered.)
  3. The hope is to release the next major version of Redux by the end of the month, so if this a breaking change (or semi-breaking change), now would be a good time to do it.
OliverJAsh commented 12 months ago

That makes sense to me. Unfortunately I don't have the time to pick this up right now, as much as I would love to. Thanks for your input and sharing your thoughts.

NoahAndrews commented 3 weeks ago

Something fancier would be cool, but the simple unknown solution would be way better than what we have now