Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent approach to singletons #9090

Closed
dbkr opened this issue Mar 8, 2019 · 3 comments
Closed

Consistent approach to singletons #9090

dbkr opened this issue Mar 8, 2019 · 3 comments
Labels
T-Task Tasks for the team like planning

Comments

@dbkr
Copy link
Member

dbkr commented Mar 8, 2019

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
Copy link
Member

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.

@lampholder lampholder added the T-Task Tasks for the team like planning label Mar 15, 2019
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this issue Jun 27, 2019
Fixes element-hq/element-web#9986

There's a few reasons for pushing this out to its own place:
* In future, we might want to move WhoIsTyping here.
* We have multiple composers now, and although they don't send typing notifications, they could (see https://github.com/vector-im/riot-web/issues/10188)
* In future we may have status for where/what the user is typing (https://github.com/matrix-org/matrix-doc/issues/437)
* The composer is complicated enough - it doesn't need to dedupe typing states too.

Note: This makes use of the principles introduced in element-hq/element-web#8923 and element-hq/element-web#9090
@bwindels
Copy link
Contributor

bwindels commented Apr 17, 2020

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
Copy link
Member

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

su-ex added a commit to SchildiChat/element-web that referenced this issue Aug 22, 2022
* Improve auth aria attributes and semantics ([\element-hq#22948](element-hq#22948)).
* Device manager - New device tile info design ([\element-hq#9122](matrix-org/matrix-react-sdk#9122)). Contributed by @kerryarchibald.
* Device manager generic settings subsection component ([\element-hq#9147](matrix-org/matrix-react-sdk#9147)). Contributed by @kerryarchibald.
* Migrate the hidden read receipts flag to new "send read receipts" option ([\element-hq#9141](matrix-org/matrix-react-sdk#9141)).
* Live location sharing - share location at most every 5 seconds ([\element-hq#9148](matrix-org/matrix-react-sdk#9148)). Contributed by @kerryarchibald.
* Increase max length of voice messages to 15m ([\element-hq#9133](matrix-org/matrix-react-sdk#9133)). Fixes element-hq#18620.
* Move pin drop out of labs ([\element-hq#9135](matrix-org/matrix-react-sdk#9135)).
* Start DM on first message ([\element-hq#8612](matrix-org/matrix-react-sdk#8612)). Fixes element-hq#14736.
* Remove "Add Space" button from RoomListHeader when user cannot create spaces ([\element-hq#9129](matrix-org/matrix-react-sdk#9129)).
* The Welcome Home Screen: Dedicated Download Apps Dialog ([\element-hq#9120](matrix-org/matrix-react-sdk#9120)). Fixes element-hq#22921. Contributed by @justjanne.
* The Welcome Home Screen: "Submit Feedback" pane ([\element-hq#9090](matrix-org/matrix-react-sdk#9090)). Fixes element-hq#22918. Contributed by @justjanne.
* New User Onboarding Task List ([\element-hq#9083](matrix-org/matrix-react-sdk#9083)). Fixes element-hq#22919. Contributed by @justjanne.
* Add support for disabling spell checking ([\element-hq#8604](matrix-org/matrix-react-sdk#8604)). Fixes element-hq#21901.
* Live location share - leave maximised map open when beacons expire ([\element-hq#9098](matrix-org/matrix-react-sdk#9098)). Contributed by @kerryarchibald.
* Some slash-commands (`/myroomnick`) have temporarily been disabled before the first message in a DM is sent. ([\element-hq#9193](matrix-org/matrix-react-sdk#9193)).
* Use stable reference for active tab in tabbedView ([\#9145](matrix-org/matrix-react-sdk#9145)). Contributed by @kerryarchibald.
* Fix pillification sometimes doubling up ([\element-hq#9152](matrix-org/matrix-react-sdk#9152)). Fixes element-hq#23036.
* Fix highlights not being applied to plaintext messages ([\element-hq#9126](matrix-org/matrix-react-sdk#9126)). Fixes element-hq#22787.
* Fix dismissing edit composer when change was undone ([\element-hq#9109](matrix-org/matrix-react-sdk#9109)). Fixes element-hq#22932.
* 1-to-1 DM rooms with bots now act like DM rooms instead of multi-user-rooms before ([\element-hq#9124](matrix-org/matrix-react-sdk#9124)). Fixes element-hq#22894.
* Apply inline start padding to selected lines on modern layout only ([\element-hq#9006](matrix-org/matrix-react-sdk#9006)). Fixes element-hq#22768. Contributed by @luixxiul.
* Peek into world-readable rooms from spotlight ([\element-hq#9115](matrix-org/matrix-react-sdk#9115)). Fixes element-hq#22862.
* Use default styling on nested numbered lists due to MD being sensitive ([\element-hq#9110](matrix-org/matrix-react-sdk#9110)). Fixes element-hq#22935.
* Fix replying using chat effect commands ([\element-hq#9101](matrix-org/matrix-react-sdk#9101)). Fixes element-hq#22824.
@dbkr dbkr closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

No branches or pull requests

4 participants