imterah/element-portable
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Store refactor: use non-global stores in components (#9293)

Browse source 
* Add Stores and StoresContext and use it in MatrixChat and RoomView

Added a new kind of class:
- Add God object `Stores` which will hold refs to all known stores and the `MatrixClient`. This object is NOT a singleton.
- Add `StoresContext` to hold onto a ref of `Stores` for use inside components.

`StoresContext` is created via:
- Create `Stores` in `MatrixChat`, assigning the `MatrixClient` when we have one set. Currently sets the RVS to `RoomViewStore.instance`.
- Wrap `MatrixChat`s `render()` function in a `StoresContext.Provider` so it can be used anywhere.

`StoresContext` is currently only used in `RoomView` via the following changes:
- Remove the HOC, which redundantly set `mxClient` as a prop. We don't need this as `RoomView` was using the client from `this.context`.
- Change the type of context accepted from `MatrixClientContext` to `StoresContext`.
- Modify alllll the places where `this.context` is used to interact with the client and suffix `.client`.
- Modify places where we use `RoomViewStore.instance` and replace them with `this.context.roomViewStore`.

This makes `RoomView` use a non-global instance of RVS.

* Linting

* SDKContext and make client an optional constructor arg

* Move SDKContext to /src/contexts

* Inject all RVS deps

* Linting

* Remove reset calls; deep copy the INITIAL_STATE to avoid test pollution

* DI singletons used in RoomView; DI them in RoomView-test too

* Initial RoomViewStore.instance after all files are imported to avoid cyclical deps

* Lazily init stores to allow for circular dependencies

Rather than stores accepting a list of other stores in their constructors,
which doesn't work when A needs B and B needs A, make new-style stores simply
accept Stores. When a store needs another store, they access it via `Stores`
which then lazily constructs that store if it needs it. This breaks the
circular dependency at constructor time, without needing to introduce
wiring diagrams or any complex DI framework.

* Delete RoomViewStore.instance

Replaced with Stores.instance.roomViewStore

* Linting

* Move OverridableStores to test/TestStores

* Rejig how eager stores get made; don't automatically do it else tests break

* Linting

* Linting and review comments

* Fix new code to use Stores.instance

* s/Stores/SdkContextClass/g

* Update docs

* Remove unused imports

* Update src/stores/RoomViewStore.tsx

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>

* Remove empty c'tor to make sonar happy

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
kegsay 2022-10-19 13:07:03 +01:00 • committed by GitHub
parent 84f2974b57
commit e946674df3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
36 changed files with 467 additions and 275 deletions
Download patch file Download diff file Expand all files Collapse all files

8
test/SlashCommands-test.tsx
View file

@ -21,9 +21,9 @@ import { Command, Commands, getCommand } from '../src/SlashCommands';
import { createTestClient } from './test-utils';
import { MatrixClientPeg } from '../src/MatrixClientPeg';
import { LocalRoom, LOCAL_ROOM_ID_PREFIX } from '../src/models/LocalRoom';
import { RoomViewStore } from '../src/stores/RoomViewStore';
import SettingsStore from '../src/settings/SettingsStore';
import LegacyCallHandler from '../src/LegacyCallHandler';
import { SdkContextClass } from '../src/contexts/SDKContext';
describe('SlashCommands', () => {
let client: MatrixClient;
@ -38,14 +38,14 @@ describe('SlashCommands', () => {
};
const setCurrentRoom = (): void => {
mocked(RoomViewStore.instance.getRoomId).mockReturnValue(roomId);
mocked(SdkContextClass.instance.roomViewStore.getRoomId).mockReturnValue(roomId);
mocked(client.getRoom).mockImplementation((rId: string): Room => {
if (rId === roomId) return room;
});
};
const setCurrentLocalRoon = (): void => {
mocked(RoomViewStore.instance.getRoomId).mockReturnValue(localRoomId);
mocked(SdkContextClass.instance.roomViewStore.getRoomId).mockReturnValue(localRoomId);
mocked(client.getRoom).mockImplementation((rId: string): Room => {
if (rId === localRoomId) return localRoom;
});
@ -60,7 +60,7 @@ describe('SlashCommands', () => {
room = new Room(roomId, client, client.getUserId());
localRoom = new LocalRoom(localRoomId, client, client.getUserId());
jest.spyOn(RoomViewStore.instance, "getRoomId");
jest.spyOn(SdkContextClass.instance.roomViewStore, "getRoomId");
});
describe('/topic', () => {

44
test/TestStores.ts Normal file
View file

@ -0,0 +1,44 @@
/*
Copyright 2022 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import { SdkContextClass } from "../src/contexts/SDKContext";
import { PosthogAnalytics } from "../src/PosthogAnalytics";
import { SlidingSyncManager } from "../src/SlidingSyncManager";
import { RoomNotificationStateStore } from "../src/stores/notifications/RoomNotificationStateStore";
import RightPanelStore from "../src/stores/right-panel/RightPanelStore";
import { RoomViewStore } from "../src/stores/RoomViewStore";
import { SpaceStoreClass } from "../src/stores/spaces/SpaceStore";
import { WidgetLayoutStore } from "../src/stores/widgets/WidgetLayoutStore";
import WidgetStore from "../src/stores/WidgetStore";
/**
* A class which provides the same API as Stores but adds additional unsafe setters which can
* replace individual stores. This is useful for tests which need to mock out stores.
*/
export class TestStores extends SdkContextClass {
public _RightPanelStore?: RightPanelStore;
public _RoomNotificationStateStore?: RoomNotificationStateStore;
public _RoomViewStore?: RoomViewStore;
public _WidgetLayoutStore?: WidgetLayoutStore;
public _WidgetStore?: WidgetStore;
public _PosthogAnalytics?: PosthogAnalytics;
public _SlidingSyncManager?: SlidingSyncManager;
public _SpaceStore?: SpaceStoreClass;
constructor() {
super();
}
}

41
test/components/structures/RoomView-test.tsx
View file

@ -32,17 +32,16 @@ import { defaultDispatcher } from "../../../src/dispatcher/dispatcher";
import { ViewRoomPayload } from "../../../src/dispatcher/payloads/ViewRoomPayload";
import { RoomView as _RoomView } from "../../../src/components/structures/RoomView";
import ResizeNotifier from "../../../src/utils/ResizeNotifier";
import { RoomViewStore } from "../../../src/stores/RoomViewStore";
import SettingsStore from "../../../src/settings/SettingsStore";
import { SettingLevel } from "../../../src/settings/SettingLevel";
import DMRoomMap from "../../../src/utils/DMRoomMap";
import { NotificationState } from "../../../src/stores/notifications/NotificationState";
import RightPanelStore from "../../../src/stores/right-panel/RightPanelStore";
import { RightPanelPhases } from "../../../src/stores/right-panel/RightPanelStorePhases";
import { LocalRoom, LocalRoomState } from "../../../src/models/LocalRoom";
import { DirectoryMember } from "../../../src/utils/direct-messages";
import { createDmLocalRoom } from "../../../src/utils/dm/createDmLocalRoom";
import { UPDATE_EVENT } from "../../../src/stores/AsyncStore";
import { SdkContextClass, SDKContext } from "../../../src/contexts/SDKContext";
const RoomView = wrapInMatrixClientContext(_RoomView);
@ -50,6 +49,7 @@ describe("RoomView", () => {
let cli: MockedObject<MatrixClient>;
let room: Room;
let roomCount = 0;
let stores: SdkContextClass;
beforeEach(async () => {
mockPlatformPeg({ reload: () => {} });
@ -64,7 +64,9 @@ describe("RoomView", () => {
room.on(RoomEvent.TimelineReset, (...args) => cli.emit(RoomEvent.TimelineReset, ...args));
DMRoomMap.makeShared();
RightPanelStore.instance.useUnitTestClient(cli);
stores = new SdkContextClass();
stores.client = cli;
stores.rightPanelStore.useUnitTestClient(cli);
});
afterEach(async () => {
@ -73,15 +75,15 @@ describe("RoomView", () => {
});
const mountRoomView = async (): Promise<ReactWrapper> => {
if (RoomViewStore.instance.getRoomId() !== room.roomId) {
if (stores.roomViewStore.getRoomId() !== room.roomId) {
const switchedRoom = new Promise<void>(resolve => {
const subFn = () => {
if (RoomViewStore.instance.getRoomId()) {
RoomViewStore.instance.off(UPDATE_EVENT, subFn);
if (stores.roomViewStore.getRoomId()) {
stores.roomViewStore.off(UPDATE_EVENT, subFn);
resolve();
}
};
RoomViewStore.instance.on(UPDATE_EVENT, subFn);
stores.roomViewStore.on(UPDATE_EVENT, subFn);
});
defaultDispatcher.dispatch<ViewRoomPayload>({
@ -94,15 +96,16 @@ describe("RoomView", () => {
}
const roomView = mount(
<RoomView
mxClient={cli}
threepidInvite={null}
oobData={null}
resizeNotifier={new ResizeNotifier()}
justCreatedOpts={null}
forceTimeline={false}
onRegistered={null}
/>,
<SDKContext.Provider value={stores}>
<RoomView
threepidInvite={null}
oobData={null}
resizeNotifier={new ResizeNotifier()}
justCreatedOpts={null}
forceTimeline={false}
onRegistered={null}
/>
</SDKContext.Provider>,
);
await act(() => Promise.resolve()); // Allow state to settle
return roomView;
@ -162,14 +165,14 @@ describe("RoomView", () => {
it("normally doesn't open the chat panel", async () => {
jest.spyOn(NotificationState.prototype, "isUnread", "get").mockReturnValue(false);
await mountRoomView();
expect(RightPanelStore.instance.isOpen).toEqual(false);
expect(stores.rightPanelStore.isOpen).toEqual(false);
});
it("opens the chat panel if there are unread messages", async () => {
jest.spyOn(NotificationState.prototype, "isUnread", "get").mockReturnValue(true);
await mountRoomView();
expect(RightPanelStore.instance.isOpen).toEqual(true);
expect(RightPanelStore.instance.currentCard.phase).toEqual(RightPanelPhases.Timeline);
expect(stores.rightPanelStore.isOpen).toEqual(true);
expect(stores.rightPanelStore.currentCard.phase).toEqual(RightPanelPhases.Timeline);
});
});

5
test/components/views/beacon/RoomCallBanner-test.tsx
View file

@ -42,8 +42,8 @@ import RoomCallBanner from "../../../../src/components/views/beacon/RoomCallBann
import { CallStore } from "../../../../src/stores/CallStore";
import { WidgetMessagingStore } from "../../../../src/stores/widgets/WidgetMessagingStore";
import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
import { RoomViewStore } from "../../../../src/stores/RoomViewStore";
import { ConnectionState } from "../../../../src/models/Call";
import { SdkContextClass } from "../../../../src/contexts/SDKContext";
describe("<RoomCallBanner />", () => {
let client: Mocked<MatrixClient>;
@ -132,7 +132,8 @@ describe("<RoomCallBanner />", () => {
});
it("doesn't show banner if the call is shown", async () => {
jest.spyOn(RoomViewStore.instance, 'isViewingCall').mockReturnValue(true);
jest.spyOn(SdkContextClass.instance.roomViewStore, "isViewingCall");
mocked(SdkContextClass.instance.roomViewStore.isViewingCall).mockReturnValue(true);
await renderBanner();
const banner = await screen.queryByText("Video call");
expect(banner).toBeFalsy();

68
test/stores/RoomViewStore-test.tsx → test/stores/RoomViewStore-test.ts
View file

@ -21,10 +21,21 @@ import { Action } from '../../src/dispatcher/actions';
import { getMockClientWithEventEmitter, untilDispatch, untilEmission } from '../test-utils';
import SettingsStore from '../../src/settings/SettingsStore';
import { SlidingSyncManager } from '../../src/SlidingSyncManager';
import { PosthogAnalytics } from '../../src/PosthogAnalytics';
import { TimelineRenderingType } from '../../src/contexts/RoomContext';
import { MatrixDispatcher } from '../../src/dispatcher/dispatcher';
import { UPDATE_EVENT } from '../../src/stores/AsyncStore';
import { ActiveRoomChangedPayload } from '../../src/dispatcher/payloads/ActiveRoomChangedPayload';
import { SpaceStoreClass } from '../../src/stores/spaces/SpaceStore';
import { TestStores } from '../TestStores';
// mock out the injected classes
jest.mock('../../src/PosthogAnalytics');
const MockPosthogAnalytics = <jest.Mock<PosthogAnalytics>><unknown>PosthogAnalytics;
jest.mock('../../src/SlidingSyncManager');
const MockSlidingSyncManager = <jest.Mock<SlidingSyncManager>><unknown>SlidingSyncManager;
jest.mock('../../src/stores/spaces/SpaceStore');
const MockSpaceStore = <jest.Mock<SpaceStoreClass>><unknown>SpaceStoreClass;
jest.mock('../../src/utils/DMRoomMap', () => {
const mock = {
@ -51,6 +62,9 @@ describe('RoomViewStore', function() {
isGuest: jest.fn(),
});
const room = new Room(roomId, mockClient, userId);
let roomViewStore: RoomViewStore;
let slidingSyncManager: SlidingSyncManager;
let dis: MatrixDispatcher;
beforeEach(function() {
@ -60,10 +74,17 @@ describe('RoomViewStore', function() {
mockClient.getRoom.mockReturnValue(room);
mockClient.isGuest.mockReturnValue(false);
// Reset the state of the store
// Make the RVS to test
dis = new MatrixDispatcher();
RoomViewStore.instance.reset();
RoomViewStore.instance.resetDispatcher(dis);
slidingSyncManager = new MockSlidingSyncManager();
const stores = new TestStores();
stores._SlidingSyncManager = slidingSyncManager;
stores._PosthogAnalytics = new MockPosthogAnalytics();
stores._SpaceStore = new MockSpaceStore();
roomViewStore = new RoomViewStore(
dis, stores,
);
stores._RoomViewStore = roomViewStore;
});
it('can be used to view a room by ID and join', async () => {
@ -71,14 +92,14 @@ describe('RoomViewStore', function() {
dis.dispatch({ action: Action.JoinRoom });
await untilDispatch(Action.JoinRoomReady, dis);
expect(mockClient.joinRoom).toHaveBeenCalledWith(roomId, { viaServers: [] });
expect(RoomViewStore.instance.isJoining()).toBe(true);
expect(roomViewStore.isJoining()).toBe(true);
});
it('can auto-join a room', async () => {
dis.dispatch({ action: Action.ViewRoom, room_id: roomId, auto_join: true });
await untilDispatch(Action.JoinRoomReady, dis);
expect(mockClient.joinRoom).toHaveBeenCalledWith(roomId, { viaServers: [] });
expect(RoomViewStore.instance.isJoining()).toBe(true);
expect(roomViewStore.isJoining()).toBe(true);
});
it('emits ActiveRoomChanged when the viewed room changes', async () => {
@ -97,7 +118,7 @@ describe('RoomViewStore', function() {
it('invokes room activity listeners when the viewed room changes', async () => {
const roomId2 = "!roomid:2";
const callback = jest.fn();
RoomViewStore.instance.addRoomListener(roomId, callback);
roomViewStore.addRoomListener(roomId, callback);
dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
await untilDispatch(Action.ActiveRoomChanged, dis) as ActiveRoomChangedPayload;
expect(callback).toHaveBeenCalledWith(true);
@ -116,14 +137,14 @@ describe('RoomViewStore', function() {
}, dis);
// roomId is set to id of the room alias
expect(RoomViewStore.instance.getRoomId()).toBe(roomId);
expect(roomViewStore.getRoomId()).toBe(roomId);
// join the room
dis.dispatch({ action: Action.JoinRoom }, true);
await untilDispatch(Action.JoinRoomReady, dis);
expect(RoomViewStore.instance.isJoining()).toBeTruthy();
expect(roomViewStore.isJoining()).toBeTruthy();
expect(mockClient.joinRoom).toHaveBeenCalledWith(alias, { viaServers: [] });
});
@ -134,7 +155,7 @@ describe('RoomViewStore', function() {
const payload = await untilDispatch(Action.ViewRoomError, dis);
expect(payload.room_id).toBeNull();
expect(payload.room_alias).toEqual(alias);
expect(RoomViewStore.instance.getRoomAlias()).toEqual(alias);
expect(roomViewStore.getRoomAlias()).toEqual(alias);
});
it('emits JoinRoomError if joining the room fails', async () => {
@ -143,8 +164,8 @@ describe('RoomViewStore', function() {
dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
dis.dispatch({ action: Action.JoinRoom });
await untilDispatch(Action.JoinRoomError, dis);
expect(RoomViewStore.instance.isJoining()).toBe(false);
expect(RoomViewStore.instance.getJoinError()).toEqual(joinErr);
expect(roomViewStore.isJoining()).toBe(false);
expect(roomViewStore.getJoinError()).toEqual(joinErr);
});
it('remembers the event being replied to when swapping rooms', async () => {
@ -154,13 +175,13 @@ describe('RoomViewStore', function() {
getRoomId: () => roomId,
};
dis.dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room });
await untilEmission(RoomViewStore.instance, UPDATE_EVENT);
expect(RoomViewStore.instance.getQuotingEvent()).toEqual(replyToEvent);
await untilEmission(roomViewStore, UPDATE_EVENT);
expect(roomViewStore.getQuotingEvent()).toEqual(replyToEvent);
// view the same room, should remember the event.
// set the highlighed flag to make sure there is a state change so we get an update event
dis.dispatch({ action: Action.ViewRoom, room_id: roomId, highlighted: true });
await untilEmission(RoomViewStore.instance, UPDATE_EVENT);
expect(RoomViewStore.instance.getQuotingEvent()).toEqual(replyToEvent);
await untilEmission(roomViewStore, UPDATE_EVENT);
expect(roomViewStore.getQuotingEvent()).toEqual(replyToEvent);
});
it('swaps to the replied event room if it is not the current room', async () => {
@ -172,18 +193,18 @@ describe('RoomViewStore', function() {
};
dis.dispatch({ action: 'reply_to_event', event: replyToEvent, context: TimelineRenderingType.Room });
await untilDispatch(Action.ViewRoom, dis);
expect(RoomViewStore.instance.getQuotingEvent()).toEqual(replyToEvent);
expect(RoomViewStore.instance.getRoomId()).toEqual(roomId2);
expect(roomViewStore.getQuotingEvent()).toEqual(replyToEvent);
expect(roomViewStore.getRoomId()).toEqual(roomId2);
});
it('removes the roomId on ViewHomePage', async () => {
dis.dispatch({ action: Action.ViewRoom, room_id: roomId });
await untilDispatch(Action.ActiveRoomChanged, dis);
expect(RoomViewStore.instance.getRoomId()).toEqual(roomId);
expect(roomViewStore.getRoomId()).toEqual(roomId);
dis.dispatch({ action: Action.ViewHomePage });
await untilEmission(RoomViewStore.instance, UPDATE_EVENT);
expect(RoomViewStore.instance.getRoomId()).toBeNull();
await untilEmission(roomViewStore, UPDATE_EVENT);
expect(roomViewStore.getRoomId()).toBeNull();
});
describe('Sliding Sync', function() {
@ -191,23 +212,22 @@ describe('RoomViewStore', function() {
jest.spyOn(SettingsStore, 'getValue').mockImplementation((settingName, roomId, value) => {
return settingName === "feature_sliding_sync"; // this is enabled, everything else is disabled.
});
RoomViewStore.instance.reset();
});
it("subscribes to the room", async () => {
const setRoomVisible = jest.spyOn(SlidingSyncManager.instance, "setRoomVisible").mockReturnValue(
const setRoomVisible = jest.spyOn(slidingSyncManager, "setRoomVisible").mockReturnValue(
Promise.resolve(""),
);
const subscribedRoomId = "!sub1:localhost";
dis.dispatch({ action: Action.ViewRoom, room_id: subscribedRoomId });
await untilDispatch(Action.ActiveRoomChanged, dis);
expect(RoomViewStore.instance.getRoomId()).toBe(subscribedRoomId);
expect(roomViewStore.getRoomId()).toBe(subscribedRoomId);
expect(setRoomVisible).toHaveBeenCalledWith(subscribedRoomId, true);
});
// Regression test for an in-the-wild bug where rooms would rapidly switch forever in sliding sync mode
it("doesn't get stuck in a loop if you view rooms quickly", async () => {
const setRoomVisible = jest.spyOn(SlidingSyncManager.instance, "setRoomVisible").mockReturnValue(
const setRoomVisible = jest.spyOn(slidingSyncManager, "setRoomVisible").mockReturnValue(
Promise.resolve(""),
);
const subscribedRoomId = "!sub1:localhost";

4
test/stores/widgets/StopGapWidgetDriver-test.ts
View file

@ -20,8 +20,8 @@ import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";
import { Direction, MatrixEvent } from "matrix-js-sdk/src/matrix";
import { Widget, MatrixWidgetType, WidgetKind, WidgetDriver, ITurnServer } from "matrix-widget-api";
import { SdkContextClass } from "../../../src/contexts/SDKContext";
import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
import { RoomViewStore } from "../../../src/stores/RoomViewStore";
import { StopGapWidgetDriver } from "../../../src/stores/widgets/StopGapWidgetDriver";
import { stubClient } from "../../test-utils";
@ -201,7 +201,7 @@ describe("StopGapWidgetDriver", () => {
beforeEach(() => { driver = mkDefaultDriver(); });
it('reads related events from the current room', async () => {
jest.spyOn(RoomViewStore.instance, 'getRoomId').mockReturnValue('!this-room-id');
jest.spyOn(SdkContextClass.instance.roomViewStore, 'getRoomId').mockReturnValue('!this-room-id');
client.relations.mockResolvedValue({
originalEvent: new MatrixEvent(),