Ensure redux reducer functions do not mutate state or parameters

[haszari]

Describe the bug:

As highlighted by @ecgan in a recent review: https://github.com/saucal/pinterest-for-woocommerce/pull/100/files/dd48da02af9a24ba2a59e86eedf11fb3a6d30447#r674148315

https://redux.js.org/style-guide/style-guide#do-not-mutate-state https://redux.js.org/usage/troubleshooting#never-mutate-reducer-arguments

Looks like both the onboarding reducer (setup-guide/app/data/settings) and the catalog sync reducer (catalog-sync/app/data/reports) both mutate state param.

Typically reducer would return a new object direct, e.g.

	switch ( action.type ) {
		case TYPES.RECEIVE_SETTINGS:
			return {
				...state,
				settings: {
					...state.settings,
					...action.settings,
				},
			};

Not sure what bugs/impact this would cause but we're playing with fire not following the recommendations.

UPDATE: (see comment below) - the state is not actually mutated, just the state param ref is re-assigned. Keeping the issue open as it's probably cleaner and more aligned with convention return a new object direct (or use a local variable).

[msaggiorato]

Hey @haszari , we're actually creating a new object, just assigning it to the input variable. The state is actually not mutated, a new one is created and then returned.

We can go ahead with this suggestion, but I don't see how this affects things to be high priority? If you can let me know, that'd be great.

To me it seems just coding style issues, which shouldn't compromise release at this point.

[haszari]

we're actually creating a new object, just assigning it to the input variable. The state is actually not mutated, a new one is created and then returned.

Right you are @msaggiorato ! I agree this is more a style issue if the actual state param is not mutated (just the ref). Reducing priority & I'll tweak the description - apologies for the noise.