[redux-subspace] In namespaced reducer, actions should be global by default

[travikk]

Currently, once a reducer is namespaced, if an action isn't specifically global it can't be processed by the reducer.

For anyone dispatching an action (which isn't specifically namespaced or made global), there's no way for a namespaced reducer to handle said action. This is problematic as it creates an obligation on the producer of the action to know whether or not it should be global and use redux-subspace specific APIs to dispatch that action.

Realistically, you can't use middleware to mark actions global by default, as in a large-scale app there could be hundreds different actions, and tracking all of them might be impossible.

Currently, this peace of code is reponsible for described behavior:

const processAction = (namespace) => (action, callback, defaultValue) => {
    if (!namespace || isGlobal(action)) {
        return callback(action)
    } else if (hasNamespace(action, namespace)) {
        return callback({...action, type: action.type.substring(namespace.length + 1)})
    } else {
        return defaultValue
    }
}

I would propose changing it to the following:

const processAction = (namespace) => (action, callback) => {
    if (hasNamespace(action, namespace)) {
        return callback({...action, type: action.type.substring(namespace.length + 1)})
    } else {
        return callback(action)
    }
}

Note: this gets rid of the necessity of defaultValue, as by Redux API it is a reducer's responsibility to return a state unchaged, when given an action that reducer does not handle.

I'm keen to hear your opinions and thoughts behind original design. Thanks

[mpeyper]

Hi @travikk,

Thanks for the question. When designing the library we essentially had the choice between:

1. isolation by default
2. isolation by exception

What redux-subspace provides is option 1 and what you are describing is option 2. The main reason why we went this way was that we wanted to restrict the amount of cross-subspace communication to a minimum (we follow the micro-frontend pattern so the more isolated a frontend is, the more portable it becomes).

By allowing any action into the reducer, you also open up the potential for subtle bugs that are difficult to debug. Take for example a reducer structure like:

const data = (state = {}, action) => action.type === 'DATA_LOADED' ? { ...state, ...action.payload } : state

const child = combineReducers({
  data
})

const parent = combineReducers({
  data
  child: namespaced('child')(child)
})

const reducer = combineReducers({
  parent: namespaced('parent')(parent)
})

Now a parent/child/DATA_LOADED action is only going to affect state.parent.child.data, and not state.parent.data. However, a parent/DATA_LOADED action will affect both because by the time it gets to the child reducer, the parent/ has been stripped away.

This gets even harder to debug when there are even more layers in play, e.g:

const data = (state = {}, action) => action.type === 'DATA_LOADED' ? { ...state, ...action.payload } : state

const child = combineReducers({
  data
})

const middle = combineReducers({
  child: namespaced('child')(child)
})

const parent = combineReducers({
  data
  middle: namespaced('middle')(middle)
})

const reducer = combineReducers({
  data
  parent: namespaced('parent')(parent),
  child: namespaced('child')(child),
})

Now

• parent/middle/child/DATA_LOADED would updatestate.parent.middle.child.data`

But

• DATA_LOADED would update
  • state.data
  • state.parent.data
  • state.parent.middle.child.data
• parent/DATA_LOADED would update
  • state.parent.data
  • state.parent.middle.child.data
• child/DATA_LOADED would update
  • state.parent.data
  • state.parent.middle.child.data
• parent/child/DATA_LOADED would update state.parent.middle.child.data
  • This one seems to be clearly wrong as it's missing the middle namespace in the action type, but because parent/ gets stripped and middle passes it on as is, child will accept it
• middle/DATA_LOADED would updatestate.parent.middle.child.data`
  • Again, this does not seem like it should be allowed, but because of passing on ans stripping, by the time it gets to the child, it's good to go

There are numerous ways to structure this same problem, whereby allowing actions into more reducers, you can accidentally create cross-talk in the state. By stopping everything at the gate by default, this is no longer an issue. The effect of this type of issue is also much more costly in a micro-frontend architecture, where each frontend can be developed, tested and deployed in independently from one another and even which app it will ultimately be running in can vary (we share some frontends between multiple apps, so the issue might only present itself in one app because of the other frontends it's running with), and it's not until they all exist in the shared runtime that the issue presents itself.

On the flip side, explicitly requiring an action to be flagged as global to allow wanted cross-subspace communication is an actionable step in the development of a feature.

Realistically, you can't use middleware to mark actions global by default, as in a large-scale app there could be hundreds different actions, and tracking all of them might be impossible.

I'd argue that you probably don't want all the action to actually be global by default. We run multiple large scale applications with hundreds of different actions and have < 10 global actions per app. I will concede that we push the isolation pretty hard (which comes with a whole other set of trade-offs) and that isn't going to be for everyone. For what it's worth, the globalActions middleware does support regex, so you might be able to flag large chunks of actions as global with a clever pattern (but I'll leave writing that one up to you 😉)

Unfortunately, I don't see us changing this behaviour any time soon without a very compelling use case or some other way of reducing accidental cross-talk.