[DO NOT MERGE] Add note's tags to history screen - Use action generator #2837
Conversation
| const revision = state.data.noteRevisions.get(noteId)?.get(version); | ||
|
|
||
| if (!revision) { | ||
| return; | ||
| } | ||
|
|
||
| const note = state.data.notes.get(noteId); | ||
| const noteEmailTags = | ||
| note?.tags.filter((tagName) => isEmailTag(tagName)) ?? []; | ||
|
|
||
| const revisionCanonicalTags = revision.tags.filter((tagName) => { | ||
| const tagHash = tagHashOf(tagName); | ||
| const hasTag = state.data.tags.has(tagHash); | ||
| return !isEmailTag(tagName) && (hasTag || includeDeletedTags); | ||
| }); | ||
|
|
||
| return { | ||
| type: 'RESTORE_NOTE_REVISION', | ||
| noteId, | ||
| note: { | ||
| ...revision, | ||
| tags: [...noteEmailTags, ...revisionCanonicalTags], | ||
| }, | ||
| }; |
There was a problem hiding this comment.
This action contains the logic that we had in the data middleware.
| if (!result || !result.type) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This gives control to the action generator to prevent the action dispatch, it could be useful if for example we determine that the Redux action shouldn't be dispatched.
|
Thanks for exploring this route. I strongly prefer avoiding this kind of change because it fundamentally breaks the Redux model that we still maintain; that is, it prevents us from having a synchronous store update with inspectable and fake-able actions. The middleware gives us a chance to perform operations on the actions that leaves a fairly clear audit trail. Once we hide computation in thunks we lose all that. |
Yeah, unfortunately this option as well as thunk actions comes with their tradeoffs. Ok, let's avoid this option, thank you very much for review 🙇 ! |
This PR shows a potential alternative for generating a Redux action based on data from the state.
The idea I introduced here is similar to Thunk actions but quite simplified just to demonstrate how it could work. Basically I added a middleware that checks if the action is a function, if so it does a call with the current state and uses the return as the action to be passed to the next middleware. This way we could dispatch functions (aka action generators) that produce an action object based on the current state.
This option could help us to free up the middlewares and migrate some logic we're doing in the middlewares to actions. One good example for this would be the
CREATE_NOTEaction, here we’re using a Redux action to generate another one based on data from the state.