Rewrite action listener middleware types for better DX #1730
Conversation
size-limit report 📦
|
|
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 d9ce0d4:
|
| <LP extends AnyActionListenerPredicate<S>>( | ||
| options: { | ||
| actionCreator?: never | ||
| type?: never | ||
| matcher?: never | ||
| predicate: LP | ||
| listener: ActionListener<AnyAction, S, D> | ||
| } & ActionListenerOptions | ||
| ): Return |
There was a problem hiding this comment.
I'll have to check on my computer to verify, but making this overload to be the first one on top should fix the state inference problem.
When I did that on my branch, I then saw a different error which I fixed using the ListenerPredicateGuardedActionType I put in there.
There was a problem hiding this comment.
What I'm seeing is an action inference problem (which might be what you meant). I just tried moving this one up, and didn't see a difference.
FYI, I tried splitting up the predicate into two different versions: one for "it just returns a boolean", and one for "it's actually a type guard", since that seemed to make TS happier.
I did already grab LPGuardedActionType from your branch over to here.
|
I will poke at this a bit further, but this is a big enough improvement I'm inclined to just comment out the one failing typetest and ship this as 0.3. (assuming the package changes work as intended as well.) |
|
I'm sorta figuring out why this is failing. The No types declared:
With explicit types:
I suppose the question is if there's any way to get the predicate to kick in even if there's no explicit args declared...? |
markerikson
force-pushed
the
feature/listener-types-improvements
branch
from
c11d8fd to
d9ce0d4
Compare
|
Going to merge this as-is. I've got some other functionality I want to work on now that the types are basically in place. |
This PR:
The major changes in API/DX are:
middleware.addListenerandaddListenerActionnow take the exact same params - anoptionsobject containing thelistenercallback andwhenoption, with five possible variations in how the middleware decides what actions will trigger the listener callback:(action, currentState, originalState) => booleanTypedAddListener<State>andTypedAddListenerAction<State>types that can be used to create "pre-typed" versions ofmiddleware.addListenerandaddListenerActioncreateListenerEntryfunction that does the real work of looking at the options and assembling a listener entrycreateActionListenerMiddleware()defaults the state tounknownaddListenerActionas one of its attached fields, which means that if you docreateActionListenerMiddleware<MyState>(), thenmiddleware.addListener/addListenerActionare pre-typed.The net results are that it should be fairly straightforward for users to ensure proper typing for their listener callbacks with the right state and action types, similar to how we define "pre-typed" hooks.
The one thing that appears to be not working atm is that a
ListenerPredicateof(action, currentState, previousState) : action is PayloadAction<number> => {}does not appear to be inferring the type ofactionin the listener callback (see test file line 813). You can see the failure in the test job run.