JS: Model Redux and related libraries

[asgerf]

Adds a model of the Redux library and related libraries in the Redux ecosystem, including React-Redux.

Quick overview of Redux

A Redux store consists of an initial state, a list of actions and a reducer function. The current state can always be computed by doing actions.reduce(reducer, initialState). The only valid way to alter the current state is to append more actions to the action list. Appending such an action is called dispatching the action. The idea is that the state of the UI as a whole is derived this current state object provided by Redux.

In the Redux model we are mainly concerned with two data flow steps:

• Flow from a dispatched action into the reducer function,
• Flow from the reducer function's return value to an access of the current state.

Some of the complications here are:

• There are many ways to create and dispatch an action.
• There are many ways to construct and compose reducer functions.
• There are many ways to access the current state.

The bulk of the model is thus to model all the ways you can do the above, in particular, handling the two primary higher-order reducers we care about: the state-delegator, and the action-delegator.

The state-delegator, commonly called combineReducers, delegates a part of the state object to another function:

let reducer = combineReducers({ foo: fooReducer, bar: barReducer })

// equivalent to:
let reducer = (state, action) => {
  return {
    foo: fooReducer(state.foo, action),
    bar: barReducer(state.bar, action),
  }
};

The action-delegator delegates to another function based on the action.type property:

let reducer = handleAction('foo', fooReducer);

// equivalent to:
let reducer = (state, action) => {
  if (action.type === 'foo') {
    return fooReducer(state, action);
  } else {
    return state;
  }
};

Much of the abstraction comes down to handling those two cases.

Program slicing

Redux doesn't have any global state, but I decided to model it globally to avoid having to model every step of the wiring between the Redux store and an action dispatch or state access. In react-redux that would require reasoning about the nesting of JSX components all the way up to the root component, and the chance of missing a step would be too high.

To avoid false flow in monorepos containing multiple web apps, we use the enclosing package.json files to indicate which app a given file belongs to, and thereby which Redux store it can access.

React-redux

React-redux is responsible for the connect(mapStateToProps, mapDispatchToProps) calls that tend to wrap every component in a react-redux app. mapStateToProps provides a way to access the current state, and mapDispatchToProps provides a way to dispatch actions, and in principle the model just needs to contribute these state accesses and actions dispatch calls to the abstractions set up by the main model.

However, there's one place where the core Redux model has a direct dependency on the react-redux model (ActionCreator.ref), which was to avoid a circular dependency with type tracking.

Evaluations

Evaluations: (internal links)

• Security on redux-apps shows a few thousand new flow steps were added.
• Security on nightly shows a modest performance cost, which I think we'll just have to accept at this point.
  • An earlier evaluation shows the performance issue which was fixed by the BasicBlock.dominates change.
• This evaluation, which includes JS: Drive-by changes from Redux modelling #5019, shows some new prototype pollution alerts that make use of the new steps (though nothing exploitable).

[erik-krogh]

It looks good, but takes a while to chew through.

The order of the classes/predicates is sometimes a bit chaotic.
It's mostly RootStateSource/rootStateAccessPath, where there is a huge distance (~700 lines) between their definitions and the first use.

I have some minor comments.

[erik-krogh]

Could this be a PreCallgraphStep, instead of only being used by redux?

[asgerf]

Maybe as a type tracking step it would make sense, e.g. for tracking class constructors through some wrapper function. For an AdditionalFlowStep it seems a little dubious to me.

I'll run an experiment to see if anything interesting happens.

[asgerf]

Thanks @erik-krogh!

The order of the classes/predicates is sometimes a bit chaotic.

Yes, sorry about that. I've shuffled things around a lot, but couldn't find a solution that didn't have this problem.

[erik-krogh]

LGTM.
But I think a change-note is needed.

[asgerf]

It looks like we hit the BDD node limit after the review changes 🤦

[asgerf]

Updated evaluations:

• Default slugs shows performance similar to the original evaluation
• Redux-specific slugs shows a couple new alerts and some more tainted nodes.