Element-web: Consistent approach to singletons

[dbkr]

We use quite a lot of singletons in react-sdk but we aren't very consistent in the approach. The most common one is:

module.exports = new Boodle();

...which is very short & simple to use (import Boodle from './boodle'; Boodle.boopityBoo()) but hard to test.

DMRoomMap has a different approach: https://github.com/matrix-org/matrix-react-sdk/blob/14b3d55a76709ed1a91751ab93fb4a24d51a4258/src/utils/DMRoomMap.js#L48

But probably the closest to a convention outside of Matrix land is what I've just done in UserActivity: https://github.com/matrix-org/matrix-react-sdk/blob/7e424ce95b84cf000820a8d4af5027a9844e6233/src/UserActivity.js#L52

Perhaps we should standardise on sharedInstance() creating one if it doesn't exist and returning that?

(Also, MatrixClientPeg does something very similar expect that it's a static class that returns an instance of a matrix client rather than an instance of itself).

[turt2live]

while doing this, it might be a good idea to switch things to singletons that aren't singletons for testing purposes. Stuff like SettingsStore is impossible to test because it exposes a bunch of static methods.

[bwindels]

Can I insert a little plea in here to not enforce the singleton-ness of our singletons? E.g. don't use global state, and rather pass an object (of which there might be only 1 instance) in where we need them (e.g. Inversion of Control).

Globally enforced singletons are part of the reason that implementing for example multi-account and a grid view of multiple rooms is a lot harder than it needs to be.

Assuming we'll ever have one instance of something is an artificial restriction that is painful to remove and something we might not want to be true in the future.

[turt2live]

@bwindels that sounds a bit like dependency injection, which I am absolutely on board with :D