-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Comments
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. |
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
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. |
@bwindels that sounds a bit like dependency injection, which I am absolutely on board with :D |
* 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.
We use quite a lot of singletons in react-sdk but we aren't very consistent in the approach. The most common one is:
...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).
The text was updated successfully, but these errors were encountered: