Change: Prefer "functional" programming style instead of the current style

Change pattern proposal: More "functional" instead of current style

"Functional" is in quotes because I'm not really proposing "Functional Programming." I'm just proposing that we adopt some of the patterns from FP that make code less prone to bugs, performance issues, and complexity.

Old Pattern

Our current pattern relies heavily on 3 practices that make code significantly more complex and defect-prone:

1. Impure operations
2. Mutable structures
3. Scope leakage

New Pattern

We should prefer (in docs, in reviews, etc.):

1. Pure, idempotent operations
2. Immutability
3. Limited scope access (with a lot of flexibility)

Advantages of switching patterns

1. Massively reduced cognitive complexity (reduced scopes, immutability, idempotent operations)
2. Fewer implicit changes == fewer bugs (immutability, idempotent operations)

Disadvantages of switching patterns

1. With immutable structures, we'll need to be more careful about how much data we're handling at once (performance, memory)

What is the impact on our existing codebase?

In theory, there doesn't need to be any active impact on the existing code.

Ideally, we change our preferences going forward with minimal code changes.

In practice, this is a sea-change, and it will have very wide-reaching effects.
For example, in one of the commits in the reference implementation, I change some Merge Request Diffs code to be immutable and idempotent. This won't work because almost all of the diffs app relies heavily on in-place mutation implicitly triggering Vuex updates. This is not a positive of the diffs app (and in fact causes performance issues), but the fact is that even a small amount of change to immutability and/or idempotency could have significant impact.

Reference implementation

https://gitlab.com/gitlab-org/gitlab/-/commits/rfc/fpish

Commits	Topics	Notes
390718	Limited-scope, Idempotency	The nature of unit testing libraries means there's a lot of scope transferal. For example wrapper - defined in the outermost closure - is available and intended to be used through successively deeper and deeper scopes. However, by extracting creation and inspection functions to the module scope, we can restrict their scope by passing in the appropriate values and setting their return values to the appropriate variable. This is a significant readability improvement: createWrapper(); depends on an implicit agreement that it will assign to the wrapper variable; wrapper = createWrapper(); includes no implicit assumptions. Similarly, the inspection methods do not assume a variable defined outside their scope; the consumer should pass in the argument to use in their operation.
265f06	Immutability, Idempotency	As noted above, this change won't "work" as-is since the nature of the Diffs app depends heavily on in-place mutations. It's unlikely that cloneDeep is our best bet for working with immutable structures. It's included here as more of an acknowledgement that deeply nested objects need to be handled appropriately (e.g. var copy = { ...someDeepObject }; isn't sufficient to guarantee no mutations on the original data). Leaning all the way into full libraries like Immutable.js may also be too much. Object.freeze - while shallow - may be useful as a built-in erroring mechanism, but only if it's applied strictly and deeply.