feat(devtools): expose tracked event payloads from withTrackedReducer#302
feat(devtools): expose tracked event payloads from withTrackedReducer#302sqrter wants to merge 4 commits into
Conversation
withTrackedReducer now forwards full event objects to updateState so Redux DevTools can show payload data, not only action names.
|
Please let's not start multiple PRs for the same topic. I am closing that one. If there is no progress in #295 in the next 7 days, you can continue there. |
|
Hi @rainerhahnekamp, thanks for your response. I created it separately simply because I didn’t want to interfere with the changes that @OleksandrOleniuk made. Besides, I wasn’t sure if I understood your comments correctly. From my perspective, it would be better to continue the discussion on the newer pull request, since the old one hasn’t had any activity for a while + changing it means more work to do than just moving on with the new one. |
|
@sqrter I've pinged @OleksandrOleniuk. Let's give him the 7 days |
rainerhahnekamp
left a comment
There was a problem hiding this comment.
Thanks, great work, just one comment.
| export function updateState<State extends object>( | ||
| stateSource: WritableStateSource<State>, | ||
| action: string, | ||
| action: string | Action, |
There was a problem hiding this comment.
careful, that could be a breaking change. because if someone has a state of type {type: string}, TypeScript could have an issue in detecting if this is now the action name or update.
so better to pass on type in with-tracked-reducer and keep updateState untouched.
There was a problem hiding this comment.
@rainerhahnekamp, thanks for the review — you're absolutely right about the breaking change risk.
I reverted updateState to action: string only, keeping the public API untouched. To still forward event payloads from withTrackedReducer, I replaced the updateState call inside the reducer with direct currentActionNames.add(event) + patchState(store, ...updaters). This way withTrackedReducer keeps showing full payloads in DevTools, but updateState stays backward compatible.
| const updaters = Array.isArray(result) ? result : [result]; | ||
|
|
||
| updateState(store, event.type, ...updaters); | ||
| currentActionNames.add(event); |
There was a problem hiding this comment.
i would really like to avoid accessing that one outside the core. are there other options?
I was thinking that we could maybe try out a branded type for action. then we could keep the action as a whole.
what do you think?
There was a problem hiding this comment.
Ok, I see, but I'm not sure whether a branded type wouldn't be an overkill, simply because the users who want to manually pass Action objects would need an explicit as Action cast or similar... I think we could just completely revert the changes in with-tracked-reducer.ts, and in updateState, the action type would again become action: string | Action,. If this tiny type change in the updateState is acceptable, I can do this.
There was a problem hiding this comment.
@rainerhahnekamp Looking back at the conversation, I realized that it's the point about the breaking change you mentioned earlier. As far as I undertant, the edge case is when State = { type: string } and the user would call updateState(store, {type:'someType', payload: 'somePayload'}, { type: 'bar' }), right?
So the question is, would it be fine if we:
- create a branded type for action, something like:
declare const __actionBrand: unique symbol;
export type Action = { type: string; [key: string]: unknown } & {
readonly [__actionBrand]: true;
};- export it from public API
The usage would be like updateState(store, { type: 'Load', id: '1' } as Action, { loading: true });
Or we could also have some helper function updateState(store, createAction('[Books] Load', { id: '1' }), { loading: true });
What do you think?
There was a problem hiding this comment.
UPD: I updated the pull request to roughly see what it might look like. Still, I added a function asAction so we don't have to cast like as unknown as Action everywhere. I haven't added anything to the docs yet.
withTrackedReducernow forwards full event objects to updateState so Redux DevTools can show payload data, not only action names.Closes #294