Skip to content
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

Fix mutation in timeline reducer #6143

Conversation

Abishekcs
Copy link
Contributor

@Abishekcs Abishekcs commented Jan 19, 2025

What this PR does

Fixes mutation in timeline reducer(s) which are handling the action RESET_TITLES & UPDATE_TITLE.

Cause Bug

For this finding the cause of bug was easy as the error message exaclty indicated where mutation was as shown below:

For RESET_TITLES:

RESET_TITLES

For UPDATE_TITLE:

UPDATE_TITLE

The spread operator doesn’t work as expected with deeply nested objects because it copies the values of the top-level properties of weeks to new addresses/references, but the inner objects retain the same addresses/references.

Fix

Resolved by using lodash cloneDeep.

  • Why the changes in TimelineHandler component?
    • In Redux:
      • Mutation: Changes are immediate since we were previously directly modifying the same object reference -
        any code holding that reference sees the changes instantly.
      • Immutable: Creates a new object reference, requiring React/Redux to process and batch the state update,
        making the changes not immediately available. Also, after resetting the timeline week titles, the saveTimeline
        was called immediately, leading to an API call right away. During this time, Redux had not yet received the
        new updates.

Note:

While testing this #6138 changes should be available😅 or else /timeline page may not work.(KEEPPING IT IN DRAFT UNTIL THEN & ALSO MANY TEST ARE FAILING FOR THIS)

Screenshots

Before:

Before.mp4

After:

After.mp4

…fore saving timeline after resetting week titles immutably

Previously, the reducer handling the 'RESET_TITLES' action was performing a mutation, which made immediate changes directly in memory. As a result, React did not require an update from the Redux store to reflect the changes. However, with immutable updates, React relies on new state information from the Redux store, which introduces a slight delay of a few seconds compared to the direct mutation approach.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant