-
Notifications
You must be signed in to change notification settings - Fork 45
Alternative Event Tracking: withTrackedReducer
#266
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
base: main
Are you sure you want to change the base?
Conversation
…racking and add tests
`withEventsTracking` is failing in some cases, wihch is due to its initial design spec. `withTrackReducer` provides a potential better alternative.
| <mat-icon matSuffix>search</mat-icon> | ||
| </mat-form-field> | ||
|
|
||
| <button mat-raised-button color="primary" (click)="addRandomBook()"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: though the demo is lowkey, I would like things like requiring a type on button elements like this. I am going about backfilling this for existing demo code.
|
I did a preliminary review for now and will give a more formal one in the evening. Looks good though. One piece missing at the moment: some sort of documentation reference |
Co-authored-by: Michael Small <33669563+michael-small@users.noreply.github.com>
|
I see it as temporarily. In the end, we need support for meta-reducers in the core... |
In that respect, I think this code/approach as-is minus the outstanding items seems legit. But with those outstanding items, I defer to @wolfmanfx on the outstanding considerations. I imagine there will be some more reviewing opportunity then. |
| import { tap } from 'rxjs/operators'; | ||
| import { updateState } from './update-state'; | ||
|
|
||
| export function withTrackedReducer<State extends object>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied the withReducer and replace the patchState with updateState.
| import { updateState } from './update-state'; | ||
|
|
||
| export function withTrackedReducer<State extends object>( | ||
| ...caseReducers: CaseReducerResult<State, any>[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, I had to use any here. The EventCreator - as it is in the original version - didn't work. I guess that this is due to different settings in the tsconfig.json.
Unless, we won't get meta reducers, I wouldn't investigate this any further. The only thing I would change is to replace any with unknown.
| ); | ||
| } | ||
|
|
||
| type EventInstance<Type extends string, Payload> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are also the original types. I copied them because they are not part of the public API.
Summary
This PR introduces
withTrackedReduceras an alternative approach to tracking reducer-based state changes in Redux DevTools. It addresses limitations in the originalwithEventsTrackingimplementation.The original implementation (based on #231) attempted to automatically track reducer-based state changes via the glitch tracker. However, this approach has fundamental limitations that cannot be resolved with the specified approach.
Several edge cases are documented in the skipped tests within
with-events-tracking.spec.ts.This PR introduces an alternative way, where end users have to use the wrapperd function
withTrackedReducerwhich calls internallyupdateStateusing the event's name.If we proceed with this approach, the following items need to be completed:
withTrackedReducerusage and when to use it vswithReducerwithEventsTrackingapproach (if applicable)withTrackedReduceris only used with stores that have devtools and glitch tracking enabled