Redux overwrites model with previous state

Question

I am currently making a sample project in AngularJs combined with Redux. I am struggling to get the mappings from the reducer working.

I have a simple input where users can set a new name together with a drop down to select a 'company'.

<input type="text" ng-model="$ctrl.single.object.name">
<select ng-change="$ctrl.getProperties()"
ng-options="option.description as option.description for option in $ctrl.list.all"
ng-model="$ctrl.single.object.company">

When the user changes the company, new properties need to be fetched in order for the user to set these properties.

function FooController($ngRedux, FooActions, BarActions) {
    this.$onInit = function () {
        this.unsubscribeCompanies = $ngRedux.connect(this.mapStateToThis, BarActions)(this);
        this.fetchCompanyList();
    };

    this.$onDestroy = function () {
        this.unsubscribeCompanies();
    };

    this.fetchCompanyList = function () {
        this.fetchCompanies().payload.then((response) => {
            this.fetchCompaniesSuccess(response.data);
        }, (error) => {
            this.fetchCompaniesError(error.data);
        });
    };

    this.getProperties = function () {
        this.fetchCompanyProperties(this.single.object.company).payload.then((response) => {
            this.fetchCompanyPropertiesSuccess(response.data);
        }, (error) => {
            this.fetchCompanyPropertiesError(error.data);
        });
    };

    this.mapStateToThis = function (state) {
        return {
            list: state.bar.list,
            single: state.bar.single
        };
    };
}

module.exports = {
    template: require('./index.html'),
    controller: ['$ngRedux', 'FooActions', 'BarActions', FooController]
}

The problem I get is that the name and the selected company are overwritten with empty values when the fetch for properties is successful. I get why the values are overwritten with empty values and I have found a way to get it working.

export const GET_COMPANIES = 'GET_COMPANIES';
export const GET_COMPANIES_SUCCESS = 'GET_COMPANIES_SUCCESS';
export const GET_COMPANIES_ERROR = 'GET_COMPANIES_ERROR';
export const GET_COMPANIES_PROPERTIES = 'GET_COMPANIES_PROPERTIES';
export const GET_COMPANIES_PROPERTIES_SUCCESS = 'GET_COMPANIES_PROPERTIES_SUCCESS';
export const GET_COMPANIES_PROPERTIES_ERROR = 'GET_COMPANIES_PROPERTIES_ERROR';

export default function BarActions($http) {
    function fetchCompanies() {
        return {
            type: GET_COMPANIES,
            payload: $http.get('api/companies')
        };
    }

    function fetchCompaniesSuccess(companies) {
        return {
            type: GET_COMPANIES_SUCCESS,
            payload: companies
        };
    }

    function fetchCompaniesError(error) {
        return {
            type: GET_COMPANIES_ERROR,
            payload: error
        };
    }
    function fetchCompanyProperties(company) {
        return {
            type: GET_COMPANIES_PROPERTIES,
            payload: $http.get(`api/company/${company}/properties`)
        };
    }

    function fetchCompanyPropertiesSuccess(properties) {
        return {
            type: GET_COMPANIES_PROPERTIES_SUCCESS,
            payload: properties
        };
    }

    function fetchCompanyPropertiesError(error) {
        return {
            type: GET_COMPANIES_PROPERTIES_ERROR,
            payload: error
        };
    }

    return {
        fetchCompanies,
        fetchCompaniesSuccess,
        fetchCompaniesError,
        fetchCompanyProperties,
        fetchCompanyPropertiesSuccess,
        fetchCompanyPropertiesError
    }
}

The way I overwrite the values in the reducer is as follows:

import { GET_COMPANIES, GET_COMPANIES_SUCCESS, GET_COMPANIES_ERROR, GET_COMPANIES_PROPERTIES, GET_COMPANIES_PROPERTIES_ERROR, GET_COMPANIES_PROPERTIES_SUCCESS } from "../actions/bar.actions";

const all = [];

const initialState = {
    list: {
        all,
        filtered: all,
        error: null,
        loading: false
    },
    single: {
        object: {},
        error: null,
        loading: false
    }
};

export function BarReducer(state = initialState, action) {
    switch (action.type) {
        case GET_COMPANIES:
            return { ...state, list: { all: [], filtered: [], error: null, loading: true } };
        case GET_COMPANIES_SUCCESS:
            return { ...state, list: { all: action.payload, filtered: action.payload, error: null, loading: false } };
        case GET_COMPANIES_ERROR:
            return { ...state, list: { all: [], filtered: [], error: action.payload.innerException, loading: false } };
        case GET_COMPANIES_PROPERTIES:
            return { ...state, single: { ...state.single, object: { ...state.single.object }, error: null, loading: true } };
        case GET_COMPANIES_PROPERTIES_SUCCESS:
            return { ...state, single: { ...state.single, object: { ...state.single.object, payloadValues: action.payload }, error: null, loading: false } };
        case GET_COMPANIES_PROPERTIES_ERROR:
            return { ...state, single: { object: null, error: action.payload.innerException, loading: false } };
        default:
            return state;
    }
}

The way I now use the spread operator in order to overwrite the old state feels dirty. I was wondering if there are any rules or guidelines to handle this issue. So far I have searched a while on internet and in specific the Redux website but I did not come cross any other solutions.

brietsparks · Accepted Answer · 2018-10-19T14:46:58.470

The breakage is likely due to the structure of the reducer. It is concerned with too many different parts of state and has to operate on deep nested objects, making it easy to accidentally mutate state. The guidelines for reducer structure say that splitting reducer state into normalized slices is the best way to go.

Try splitting your one reducer into multiple smaller reducers. For example:

export const all = (initialAll = [], { type, companies }) => {
    switch(type) {
        case GET_COMPANIES_SUCCESS: return companies;
        default: return initialAll;
    }
}

export const error = (initialError = '', { type, error }) => {
    switch(type) {
        case GET_COMPANIES_ERROR: return error;
        default: return initialError;
    }
}

export const isFetching = (isFetching = false, { type }) => {
    switch(type) {
        case GET_COMPANIES: return true;
        case GET_COMPANIES_SUCCESS: return false;
        case GET_COMPANIES_ERROR: return false;
        default: return isFetching;
    }
}

Then, compose them into one reducer:

import { combineReducers } from 'redux';

export list = combineReducers({
    all,
    error,
    isFetching
});

// ...
export rootReducer = combineReducers({
    list,
    single,
    // ...
})

This way, each reducer is concerned with only one thing or set of things, and its reduction handlers can do simple operations on single-level state instead of complex operations on deep nested state.

Also, in your list substate, it looks like you are storing the same type of collection resources in both all and filtered with potential overlap. This leads to multiple sources of truth for the same data, which opens the door to data inconsistency. Instead, keep an array of filteredIds:

export const filteredIds = (initialIds = [], { type, filteredIds }) => {
    switch(type) {
        case SET_FILTERED_IDS: return filteredIds;
        default: return initialIds;
    }
}

Then, use a selector that filters all by the filteredIds to get your filtered items.

score 0 · Answer 2 · answered Apr 05 '18 at 14:27

One option is to use Immutable, which would change your reducers to:

case GET_COMPANIES:
    return state.setIn(['list', 'loading'], true);
// etc

See Using Immutable.JS with Redux for more information about this approach.

Another option is to use Lodash, as shown in this Issue, you can define the following function to make it similar to the immutable one:

import {clone, setWith, curry} from 'lodash/fp';

export const setIn = curry((path, value, obj) =>
  setWith(clone, path, value, clone(obj)),
);

Then you can use setIn as follow:

case GET_COMPANIES:
    return setIn(['list', 'loading'], true, state);
// etc

The Lodash approach is just working with plain object, so it might be easier to understand than Immutable.

Redux overwrites model with previous state

2 Answers2