From 04532f3b53b31bbd329168c212695cbf04105ae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 25 Apr 2024 10:08:04 +0200 Subject: [PATCH 1/9] Refactor tests that depended on workspace service state Those tests explicitly set the entire WorkspacesService state and would have to be modified each time we added something to the state. There's no reason for them to know about WorkspacesService state, so this commit refactors them out of that knowledge. --- .../teleterm/src/ui/TabHost/TabHost.test.tsx | 132 ++++-------------- .../src/ui/TabHost/useTabShortcuts.test.tsx | 96 +++++-------- 2 files changed, 59 insertions(+), 169 deletions(-) diff --git a/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx b/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx index f34b3ca7c9143..0f5f336b8852b 100644 --- a/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx +++ b/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx @@ -17,26 +17,13 @@ */ import { fireEvent, render, screen } from 'design/utils/testing'; -import React from 'react'; import { TabHost } from 'teleterm/ui/TabHost/TabHost'; import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; -import { - Document, - DocumentsService, - WorkspacesService, -} from 'teleterm/ui/services/workspacesService'; -import { KeyboardShortcutsService } from 'teleterm/ui/services/keyboardShortcuts'; -import { - MainProcessClient, - RuntimeSettings, - TabContextMenuOptions, -} from 'teleterm/mainProcess/types'; -import { ClustersService } from 'teleterm/ui/services/clusters'; -import AppContext from 'teleterm/ui/appContext'; +import { Document } from 'teleterm/ui/services/workspacesService'; +import { TabContextMenuOptions } from 'teleterm/mainProcess/types'; import { makeDocumentCluster } from 'teleterm/ui/services/workspacesService/documentsService/testHelpers'; - -import { getEmptyPendingAccessRequest } from '../services/workspacesService/accessRequestsService'; +import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; // TODO(ravicious): Remove the mock once a separate entry point for e-teleterm is created. // @@ -62,97 +49,32 @@ function getMockDocuments(): Document[] { ]; } -function getTestSetup({ documents }: { documents: Document[] }) { - const keyboardShortcutsService: Partial = { - subscribeToEvents() {}, - unsubscribeFromEvents() {}, - // @ts-expect-error we don't provide entire config - getShortcutsConfig() { - return { - closeTab: 'Command-W', - newTab: 'Command-T', - openSearchBar: 'Command-K', - openConnections: 'Command-P', - openClusters: 'Command-E', - openProfiles: 'Command-I', - }; - }, - }; - - const mainProcessClient: Partial = { - openTabContextMenu: jest.fn(), - getRuntimeSettings: () => ({}) as RuntimeSettings, - }; +const rootClusterUri = '/clusters/test_uri'; - const docsService: Partial = { - getDocuments(): Document[] { - return documents; - }, - getActive() { - return documents[0]; - }, - close: jest.fn(), - open: jest.fn(), - add: jest.fn(), - closeOthers: jest.fn(), - closeToRight: jest.fn(), - openNewTerminal: jest.fn(), - swapPosition: jest.fn(), - createClusterDocument: jest.fn(), - duplicatePtyAndActivate: jest.fn(), - }; - - const clustersService: Partial = { - subscribe: jest.fn(), - unsubscribe: jest.fn(), - findRootClusterByResource: jest.fn(), - findCluster: jest.fn(), - findGateway: jest.fn(), - }; +function getTestSetup({ documents }: { documents: Document[] }) { + const appContext = new MockAppContext(); + jest.spyOn(appContext.mainProcessClient, 'openTabContextMenu'); + + appContext.workspacesService.setState(draft => { + draft.rootClusterUri = rootClusterUri; + draft.workspaces[rootClusterUri] = { + documents, + location: documents[0]?.uri, + localClusterUri: rootClusterUri, + accessRequests: undefined, + }; + }); - const workspacesService: Partial = { - isDocumentActive(documentUri: string) { - return documentUri === documents[0].uri; - }, - getRootClusterUri() { - return '/clusters/test_uri'; - }, - getWorkspaces() { - return {}; - }, - getActiveWorkspace() { - return { - accessRequests: { - assumed: {}, - isBarCollapsed: false, - pending: getEmptyPendingAccessRequest(), - }, - documents, - location: undefined, - localClusterUri: '/clusters/test_uri', - }; - }, - // @ts-expect-error - using mocks - getActiveWorkspaceDocumentService() { - return docsService; - }, - useState: jest.fn(), - state: { - workspaces: {}, - rootClusterUri: '/clusters/test_uri', - }, - }; + const docsService = + appContext.workspacesService.getActiveWorkspaceDocumentService(); - const appContext: AppContext = { - // @ts-expect-error - using mocks - keyboardShortcutsService, - // @ts-expect-error - using mocks - mainProcessClient, - // @ts-expect-error - using mocks - clustersService, - // @ts-expect-error - using mocks - workspacesService, - }; + jest.spyOn(docsService, 'add'); + jest.spyOn(docsService, 'open'); + jest.spyOn(docsService, 'close'); + jest.spyOn(docsService, 'swapPosition'); + jest.spyOn(docsService, 'closeOthers'); + jest.spyOn(docsService, 'closeToRight'); + jest.spyOn(docsService, 'duplicatePtyAndActivate'); const utils = render( @@ -163,7 +85,7 @@ function getTestSetup({ documents }: { documents: Document[] }) { return { ...utils, docsService, - mainProcessClient, + mainProcessClient: appContext.mainProcessClient, }; } diff --git a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx index bd772730b9d31..f96372ca2df79 100644 --- a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx +++ b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx @@ -16,26 +16,18 @@ * along with this program. If not, see . */ -import React, { PropsWithChildren } from 'react'; +import { PropsWithChildren } from 'react'; import renderHook from 'design/utils/renderHook'; import { useTabShortcuts } from 'teleterm/ui/TabHost/useTabShortcuts'; -import { - Document, - DocumentsService, -} from 'teleterm/ui/services/workspacesService/documentsService'; +import { Document } from 'teleterm/ui/services/workspacesService/documentsService'; import { KeyboardShortcutEvent, KeyboardShortcutEventSubscriber, - KeyboardShortcutsService, } from 'teleterm/ui/services/keyboardShortcuts'; import AppContextProvider from 'teleterm/ui/appContextProvider'; -import { WorkspacesService } from 'teleterm/ui/services/workspacesService'; -import AppContext from 'teleterm/ui/appContext'; - import { makeDocumentCluster } from 'teleterm/ui/services/workspacesService/documentsService/testHelpers'; - -import { getEmptyPendingAccessRequest } from '../services/workspacesService/accessRequestsService'; +import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; function getMockDocuments(): Document[] { return [ @@ -95,69 +87,45 @@ function getMockDocuments(): Document[] { ]; } +const rootClusterUri = '/clusters/test_uri'; + function getTestSetup({ documents }: { documents: Document[] }) { + const appContext = new MockAppContext(); + let eventEmitter: KeyboardShortcutEventSubscriber; - const keyboardShortcutsService: Partial = { - subscribeToEvents(subscriber: KeyboardShortcutEventSubscriber) { + jest + .spyOn(appContext.keyboardShortcutsService, 'subscribeToEvents') + .mockImplementation((subscriber: KeyboardShortcutEventSubscriber) => { eventEmitter = subscriber; - }, - unsubscribeFromEvents() { + }); + jest + .spyOn(appContext.keyboardShortcutsService, 'unsubscribeFromEvents') + .mockImplementation(() => { eventEmitter = null; - }, - }; + }); - // @ts-expect-error - using mocks - const docsService: DocumentsService = { - getDocuments(): Document[] { - return documents; - }, - getActive() { - return documents[0]; - }, - close: jest.fn(), - open: jest.fn(), - add: jest.fn(), - closeOthers: jest.fn(), - closeToRight: jest.fn(), - openNewTerminal: jest.fn(), - swapPosition: jest.fn(), - duplicatePtyAndActivate: jest.fn(), - }; + appContext.workspacesService.setState(draft => { + draft.rootClusterUri = rootClusterUri; + draft.workspaces[rootClusterUri] = { + documents, + location: documents[0]?.uri, + localClusterUri: rootClusterUri, + accessRequests: undefined, + }; + }); - const workspacesService: Partial = { - getActiveWorkspaceDocumentService() { - return docsService; - }, - getActiveWorkspace() { - return { - accessRequests: { - assumed: {}, - isBarCollapsed: false, - pending: getEmptyPendingAccessRequest(), - }, - localClusterUri: '/clusters/test_uri', - documents: [], - location: '/docs/1', - }; - }, - useState: jest.fn(), - state: { - workspaces: {}, - rootClusterUri: '/clusters/test_uri', - }, - }; + const docsService = + appContext.workspacesService.getActiveWorkspaceDocumentService(); + + jest.spyOn(docsService, 'open'); + jest.spyOn(docsService, 'close'); + jest.spyOn(docsService, 'add'); - const appContext: AppContext = { - // @ts-expect-error - using mocks - keyboardShortcutsService, - // @ts-expect-error - using mocks - workspacesService, - }; renderHook( () => useTabShortcuts({ documentsService: docsService, - localClusterUri: workspacesService.getActiveWorkspace().localClusterUri, + localClusterUri: rootClusterUri, }), { wrapper: (props: PropsWithChildren) => ( @@ -171,7 +139,7 @@ function getTestSetup({ documents }: { documents: Document[] }) { return { emitKeyboardShortcutEvent: eventEmitter, docsService, - keyboardShortcutsService, + keyboardShortcutsService: appContext.keyboardShortcutsService, }; } From 9f1a8d8c2a194b90a428283a2791631ddc4bf353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 23 Apr 2024 16:49:49 +0200 Subject: [PATCH 2/9] Add useAppState --- .../src/ui/hooks/useAppState.test.tsx | 118 ++++++++++++++++++ .../teleterm/src/ui/hooks/useAppState.ts | 108 ++++++++++++++++ .../statePersistenceService.ts | 13 +- 3 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 web/packages/teleterm/src/ui/hooks/useAppState.test.tsx create mode 100644 web/packages/teleterm/src/ui/hooks/useAppState.ts diff --git a/web/packages/teleterm/src/ui/hooks/useAppState.test.tsx b/web/packages/teleterm/src/ui/hooks/useAppState.test.tsx new file mode 100644 index 0000000000000..ddcb27da7d9a7 --- /dev/null +++ b/web/packages/teleterm/src/ui/hooks/useAppState.test.tsx @@ -0,0 +1,118 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { render, screen, act } from 'design/utils/testing'; + +import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; +import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; + +import { useAppState } from './useAppState'; + +it('propagates changes coming from the same useAppState invocation', () => { + const appContext = new MockAppContext(); + render( + + + + ); + + act(() => { + screen.getByText('Increase').click(); + }); + + expect(screen.getByText('Counter: 1')).toBeInTheDocument(); + expect(appContext.statePersistenceService.getState()['counter']).toEqual(1); +}); + +it('reads initial state from persisted state', () => { + const appContext = new MockAppContext(); + appContext.statePersistenceService.putState({ boolean: false } as any); + + render( + + + + ); + + expect(screen.getByText('Boolean: false')).toBeInTheDocument(); +}); + +it('updates only the given key', () => { + const appContext = new MockAppContext(); + appContext.statePersistenceService.putState({ foo: 'bar' } as any); + + render( + + + + ); + + act(() => { + screen.getByText('Increase').click(); + }); + + expect(screen.getByText('Counter: 1')).toBeInTheDocument(); + expect(appContext.statePersistenceService.getState()['foo']).toEqual('bar'); +}); + +// TODO(ravicious): Change the behavior of useAppState so it actually does propagate changes across +// callsites. +it('does not propagate changes across different useAppState invocations', () => { + const appContext = new MockAppContext(); + render( + + + + + ); + + act(() => { + screen.getAllByText('Increase')[0].click(); + }); + + expect(screen.getByText('Counter: 1')).toBeInTheDocument(); + expect(screen.getByText('Counter: 0')).toBeInTheDocument(); + expect(appContext.statePersistenceService.getState()['counter']).toEqual(1); +}); + +type TestState = { counter: number; boolean: boolean }; + +const Counter = () => { + const [counter, setCounter] = useAppState<'counter', TestState>('counter', 0); + + return ( +
+ Counter: {counter} + +
+ ); +}; + +const Boolean = () => { + const [boolean, setBoolean] = useAppState<'boolean', TestState>( + 'boolean', + true + ); + + return ( +
+ Boolean: {boolean.toString()} + +
+ ); +}; diff --git a/web/packages/teleterm/src/ui/hooks/useAppState.ts b/web/packages/teleterm/src/ui/hooks/useAppState.ts new file mode 100644 index 0000000000000..67eed048d2b73 --- /dev/null +++ b/web/packages/teleterm/src/ui/hooks/useAppState.ts @@ -0,0 +1,108 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { useCallback, useState } from 'react'; + +import { useAppContext } from 'teleterm/ui/appContextProvider'; +import { StatePersistenceState } from 'teleterm/ui/services/statePersistence'; + +/** + * useAppState is like useState, but it persists the state to app_state.json under the given key. + * + * IMPORTANT: Currently, useAppState doesn't propagate changes across several callsites. That is, if + * two callsites use the same key, calling setState in one component will not cause the other + * component to update. + * + * This will _not_ work as expected: + * + * const Counter = () => { + * const [count, setCount] = useAppState('count', 0); + * + * return ( + *
+ * {count} + * + *
+ * ); + * } + * + * () => { + * return ( + * <> + * + * + * + * ); + * } + * + * However, this will work as expected: + * + * @example + * const Counter = ({count, setCount}) => { + * return ( + *
+ * {count} + * + *
+ * ); + * } + * + * () => { + * const [count, setCount] = useAppState('count', 0); + * + * return ( + * <> + * + * + * + * ); + * } + */ +export function useAppState< + // key could've been any string, but in lieu of avoiding typos, it's better to take it + // from one central definition. + Key extends keyof WholeState, + // WholeState is purely for testing purposes to replace StatePersistenceState in tests. + WholeState extends object = StatePersistenceState, +>( + key: Key, + initialState: WholeState[Key] +): [WholeState[Key], (newState: WholeState[Key]) => void] { + const { statePersistenceService } = useAppContext(); + const wholeState = statePersistenceService.getState() as WholeState; + const state = Object.hasOwn(wholeState, key) ? wholeState[key] : initialState; + // TODO(ravicious): useAppState currently doesn't propagate changes across several callsites. + // + // useAppState should either use useSyncExternalStore or some other solution to register a + // listener in statePersistenceService that gets called whenever the given key gets updated. + const [, rerender] = useState(); + + const setState = useCallback( + (newState: WholeState[Key]) => { + statePersistenceService.putState({ + ...statePersistenceService.getState(), + [key]: newState, + }); + + rerender({}); + }, + [key, statePersistenceService] + ); + + return [state, setState]; +} diff --git a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts index 3b6703969022d..1d6e63a2f9ef6 100644 --- a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts +++ b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts @@ -35,13 +35,15 @@ export type WorkspacesPersistedState = Omit & { workspaces: Record>; }; -interface StatePersistenceState { +export interface StatePersistenceState { connectionTracker: ConnectionTrackerState; workspacesState: WorkspacesPersistedState; shareFeedback: ShareFeedbackState; usageReporting: UsageReportingState; + vnet: { autoStart: boolean }; } +// Before adding new methods to this service, consider using useAppState instead. export class StatePersistenceService { constructor(private _fileStorage: FileStorage) {} @@ -93,8 +95,11 @@ export class StatePersistenceService { return this.getState().usageReporting; } - private getState(): StatePersistenceState { - const defaultState: StatePersistenceState = { + getState(): StatePersistenceState { + // Some legacy callsites expected StatePersistenceService to manage the default state for them, + // but with the move towards useAppState, we should put the default state close to where it's + // going to be used. Hence the use of Partial here. + const defaultState: Partial = { connectionTracker: { connections: [], }, @@ -114,7 +119,7 @@ export class StatePersistenceService { }; } - private putState(state: StatePersistenceState): void { + putState(state: StatePersistenceState): void { this._fileStorage.put('state', state); } } From 927c6724e6bfe5160d9ee27da978a24ea0540198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Tue, 23 Apr 2024 16:50:06 +0200 Subject: [PATCH 3/9] Automatically start VNet --- .../teleterm/src/ui/Vnet/vnetContext.test.tsx | 120 ++++++++++++++++++ .../teleterm/src/ui/Vnet/vnetContext.tsx | 27 +++- 2 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx diff --git a/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx b/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx new file mode 100644 index 0000000000000..a770b2d48ac9e --- /dev/null +++ b/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx @@ -0,0 +1,120 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { PropsWithChildren } from 'react'; +import { renderHook, waitFor, act } from '@testing-library/react'; + +import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; +import { IAppContext } from 'teleterm/ui/types'; +import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; +import { MockedUnaryCall } from 'teleterm/services/tshd/cloneableClient'; + +import { VnetContextProvider, useVnetContext } from './vnetContext'; + +describe('autostart', () => { + it('starts VNet if turned on', async () => { + const appContext = new MockAppContext(); + appContext.statePersistenceService.putState({ + ...appContext.statePersistenceService.getState(), + vnet: { autoStart: true }, + }); + + const { result } = renderHook(() => useVnetContext(), { + wrapper: createWrapper(Wrapper, { appContext }), + }); + + await waitFor( + () => expect(result.current.startAttempt.status).toEqual('success'), + { interval: 5 } + ); + }); + + it('does not start VNet if turned off', async () => { + const appContext = new MockAppContext(); + appContext.statePersistenceService.putState({ + ...appContext.statePersistenceService.getState(), + vnet: { autoStart: false }, + }); + + const { result } = renderHook(() => useVnetContext(), { + wrapper: createWrapper(Wrapper, { appContext }), + }); + + expect(result.current.startAttempt.status).toEqual(''); + }); + + it('switches off if start fails', async () => { + const appContext = new MockAppContext(); + const { statePersistenceService } = appContext; + statePersistenceService.putState({ + ...statePersistenceService.getState(), + vnet: { autoStart: true }, + }); + jest + .spyOn(appContext.vnet, 'start') + .mockRejectedValue(new MockedUnaryCall({})); + + const { result } = renderHook(() => useVnetContext(), { + wrapper: createWrapper(Wrapper, { appContext }), + }); + + await waitFor( + () => expect(result.current.startAttempt.status).toEqual('error'), + { interval: 5 } + ); + + expect(statePersistenceService.getState().vnet.autoStart).toEqual(false); + }); + + test('starting and stopping VNet toggles autostart', async () => { + const appContext = new MockAppContext(); + const { statePersistenceService } = appContext; + const { result } = renderHook(() => useVnetContext(), { + wrapper: createWrapper(Wrapper, { appContext }), + }); + + expect(statePersistenceService.getState()?.vnet?.autoStart).not.toBe(true); + + let err: Error; + + await act(async () => { + [, err] = await result.current.start(); + }); + expect(err).toBeFalsy(); + expect(statePersistenceService.getState().vnet.autoStart).toEqual(true); + + await act(async () => { + [, err] = await result.current.stop(); + }); + expect(err).toBeFalsy(); + expect(statePersistenceService.getState().vnet.autoStart).toEqual(false); + }); +}); + +const Wrapper = (props: PropsWithChildren<{ appContext: IAppContext }>) => ( + + {props.children} + +); + +//testing-library.com/docs/react-testing-library/api/#renderhook-options-initialprops +const createWrapper = (Wrapper, props) => { + return function CreatedWrapper({ children }) { + return {children}; + }; +}; diff --git a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx index 7fd2f23442b46..12c856b737532 100644 --- a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx +++ b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx @@ -24,10 +24,12 @@ import { useState, useCallback, useMemo, + useEffect, } from 'react'; import { useAsync, Attempt } from 'shared/hooks/useAsync'; import { useAppContext } from 'teleterm/ui/appContextProvider'; +import { useAppState } from 'teleterm/ui/hooks/useAppState'; /** * VnetContext manages the VNet instance. @@ -53,6 +55,9 @@ export const VnetContext = createContext(null); export const VnetContextProvider: FC = props => { const [status, setStatus] = useState('stopped'); const { vnet, mainProcessClient, configService } = useAppContext(); + const [{ autoStart }, setAppState] = useAppState('vnet', { + autoStart: false, + }); const isSupported = useMemo( () => @@ -69,16 +74,34 @@ export const VnetContextProvider: FC = props => { // Reconsider this only once the VNet daemon gets added. await vnet.start({}); setStatus('running'); - }, [vnet]) + setAppState({ autoStart: true }); + }, [vnet, setAppState]) ); const [stopAttempt, stop] = useAsync( useCallback(async () => { await vnet.stop({}); setStatus('stopped'); - }, [vnet]) + setAppState({ autoStart: false }); + }, [vnet, setAppState]) ); + useEffect(() => { + const handleAutoStart = async () => { + if (isSupported && autoStart && startAttempt.status === '') { + const [, error] = await start(); + + // Turn off autostart if starting fails. Otherwise the user wouldn't be able to turn off + // autostart by themselves. + if (error) { + setAppState({ autoStart: false }); + } + } + }; + + handleAutoStart(); + }, []); + return ( Date: Thu, 25 Apr 2024 10:12:11 +0200 Subject: [PATCH 4/9] subscribeWithSelector: Return unsubscribe function The unsubscribe function will be needed in order to comply with useSyncExternalStore API in the next commit. --- .../immutableStore/immutableStore.test.ts | 21 +++++++++++++++++++ .../services/immutableStore/immutableStore.ts | 14 ++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.test.ts b/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.test.ts index 985f392dcfaa7..7142093b41d19 100644 --- a/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.test.ts +++ b/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.test.ts @@ -43,6 +43,27 @@ describe('subscribeWithSelector', () => { expect(barUpdatedCallback).toHaveBeenCalledTimes(1); }); + it('returns a function which unsubscribes', () => { + const store = new TestStore(); + + const fooUpdatedCallback1 = jest.fn(); + store.subscribeWithSelector(state => state.foo, fooUpdatedCallback1); + + const fooUpdatedCallback2 = jest.fn(); + const unsubscribe = store.subscribeWithSelector( + state => state.foo, + fooUpdatedCallback2 + ); + unsubscribe(); + + store.setState(draft => { + draft.foo.set('lorem', 'ipsum'); + }); + + expect(fooUpdatedCallback1).toHaveBeenCalledTimes(1); + expect(fooUpdatedCallback2).not.toHaveBeenCalled(); + }); + it('calls the callbacks if multiple parts of the state get updated at the same time', () => { const store = new TestStore(); diff --git a/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.ts b/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.ts index 02913b3ce8c40..972cbbae6523c 100644 --- a/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.ts +++ b/web/packages/teleterm/src/ui/services/immutableStore/immutableStore.ts @@ -47,14 +47,16 @@ export class ImmutableStore extends Store { /** * Adds a callback which gets called only when the part of the state returned by selector is * changed. selector must be pure. + * + * Returns a functions that removes the subscription. */ subscribeWithSelector( selector: (state: T) => SelectedState, callback: () => void - ) { + ): () => void { let selectedState = selector(this.state); - this.subscribe(() => { + const subscription = () => { const newSelectedState = selector(this.state); // It doesn't appear to be explicitly documented anywhere, but Immer preserves object // identity, so Object.is works as expected. This behavior is covered by our tests. @@ -68,6 +70,12 @@ export class ImmutableStore extends Store { } selectedState = newSelectedState; - }); + }; + + this.subscribe(subscription); + + return () => { + this.unsubscribe(subscription); + }; } } From 20a687463f45aa7c6ec1d4ea01b706048ec87bd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 25 Apr 2024 10:56:16 +0200 Subject: [PATCH 5/9] Add isInitialized to WorkspacesService --- .../statePersistenceService.ts | 5 +++- .../workspacesService.test.ts | 8 +++++++ .../workspacesService/workspacesService.ts | 23 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts index 1d6e63a2f9ef6..cb88456eca51a 100644 --- a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts +++ b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts @@ -31,7 +31,10 @@ interface UsageReportingState { askedForUserJobRole: boolean; } -export type WorkspacesPersistedState = Omit & { +export type WorkspacesPersistedState = Omit< + WorkspacesState, + 'workspaces' | 'isInitialized' +> & { workspaces: Record>; }; diff --git a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts index e5cd7156379a1..39958e6d36290 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.test.ts @@ -63,7 +63,11 @@ describe('restoring workspace', () => { persistedWorkspaces: { [cluster.uri]: testWorkspace }, }); + expect(workspacesService.state.isInitialized).toEqual(false); + await workspacesService.restorePersistedState(); + + expect(workspacesService.state.isInitialized).toEqual(true); expect(workspacesService.getWorkspaces()).toStrictEqual({ [cluster.uri]: { accessRequests: { @@ -98,7 +102,11 @@ describe('restoring workspace', () => { persistedWorkspaces: {}, }); + expect(workspacesService.state.isInitialized).toEqual(false); + await workspacesService.restorePersistedState(); + + expect(workspacesService.state.isInitialized).toEqual(true); expect(workspacesService.getWorkspaces()).toStrictEqual({ [cluster.uri]: { accessRequests: { diff --git a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts index 6c94dc2497e21..ce50e64849cb4 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/workspacesService.ts @@ -65,6 +65,24 @@ import { export interface WorkspacesState { rootClusterUri?: RootClusterUri; workspaces: Record; + /** + * isInitialized signifies whether WorkspacesState has finished state restoration during the start + * of the app. It is useful in places that want to wait for the state to be restored before + * proceeding. + * + * If during the previous start of the app the user was logged into a workspace which cert has + * since expired, isInitialized will be set to true only _after_ the user logs in to that + * workspace (or closes the login modal). + * + * This field is not persisted to disk. + * + * Side note: Arguably, depending on the use case, the moment isInitialized is set to true could + * be changed to happen right before the modal is shown. Ultimately, the thing that interests us + * the most is whether the state from disk was loaded into memory. Maybe in the future we will + * need to separate values or an enum. + * + */ + isInitialized: boolean; } export interface Workspace { @@ -94,6 +112,7 @@ export class WorkspacesService extends ImmutableStore { state: WorkspacesState = { rootClusterUri: undefined, workspaces: {}, + isInitialized: false, }; constructor( @@ -403,6 +422,10 @@ export class WorkspacesService extends ImmutableStore { if (persistedState.rootClusterUri) { await this.setActiveWorkspace(persistedState.rootClusterUri); } + + this.setState(draft => { + draft.isInitialized = true; + }); } // TODO(gzdunek): Parse the entire workspace state read from disk like below. From 40edc18ebd98b7aabfb9cc24aafbf0ec3ecaf5f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 25 Apr 2024 10:59:36 +0200 Subject: [PATCH 6/9] Add useImmutableStore --- .../src/ui/hooks/useImmutableStore.ts | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 web/packages/teleterm/src/ui/hooks/useImmutableStore.ts diff --git a/web/packages/teleterm/src/ui/hooks/useImmutableStore.ts b/web/packages/teleterm/src/ui/hooks/useImmutableStore.ts new file mode 100644 index 0000000000000..ac24cfeb007f0 --- /dev/null +++ b/web/packages/teleterm/src/ui/hooks/useImmutableStore.ts @@ -0,0 +1,67 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { useSyncExternalStore, useCallback } from 'react'; + +import { useAppContext } from 'teleterm/ui/appContextProvider'; +import { IAppContext } from 'teleterm/ui/types'; +import { ImmutableStore } from 'teleterm/ui/services/immutableStore'; + +/** + * useImmutableStore selects a value out of a store and triggers component update whenever that + * value changes. The selector needs to have stable identity. + * + * @example + * const isInitialized = useImmutableStore( + * 'workspacesService', + * useCallback(state => state.isInitialized, []) + * ); + * + * @example + * // Defined outside of a component. + * const getIsInitialized = (selector: WorkspacesState) => state.isInitialized + * + * // Defined inside a React component. + * () => { + * const isInitialized = useImmutableStore('workspacesService', getIsInitialized); + * } + */ +export const useImmutableStore = < + SelectedState, + StoreKey extends ImmutableStoreKeys, +>( + storeKey: StoreKey, + selector: (state: IAppContext[StoreKey]['state']) => SelectedState +): SelectedState => { + const store = useAppContext()[storeKey]; + + const subscribe = useCallback( + (listener: () => void) => store.subscribeWithSelector(selector, listener), + [store, selector] + ); + const getSnapshot = useCallback( + () => selector(store.state), + [store, selector] + ); + + return useSyncExternalStore(subscribe, getSnapshot); +}; + +type ImmutableStoreKeys = { + [K in keyof T]: T[K] extends ImmutableStore ? K : never; +}[keyof T]; From 492f7cb23330717b6146b57618b056b188a77d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 25 Apr 2024 11:00:02 +0200 Subject: [PATCH 7/9] Wait for workspace init before VNet autostart --- .../teleterm/src/ui/Vnet/vnetContext.test.tsx | 37 +++++++++++++++++++ .../teleterm/src/ui/Vnet/vnetContext.tsx | 14 ++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx b/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx index a770b2d48ac9e..bf42c27fd3120 100644 --- a/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx +++ b/web/packages/teleterm/src/ui/Vnet/vnetContext.test.tsx @@ -28,6 +28,26 @@ import { VnetContextProvider, useVnetContext } from './vnetContext'; describe('autostart', () => { it('starts VNet if turned on', async () => { + const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.isInitialized = true; + }); + appContext.statePersistenceService.putState({ + ...appContext.statePersistenceService.getState(), + vnet: { autoStart: true }, + }); + + const { result } = renderHook(() => useVnetContext(), { + wrapper: createWrapper(Wrapper, { appContext }), + }); + + await waitFor( + () => expect(result.current.startAttempt.status).toEqual('success'), + { interval: 5 } + ); + }); + + it('waits for workspace state to be initialized', async () => { const appContext = new MockAppContext(); appContext.statePersistenceService.putState({ ...appContext.statePersistenceService.getState(), @@ -38,6 +58,14 @@ describe('autostart', () => { wrapper: createWrapper(Wrapper, { appContext }), }); + expect(result.current.startAttempt.status).toEqual(''); + + act(() => { + appContext.workspacesService.setState(draft => { + draft.isInitialized = true; + }); + }); + await waitFor( () => expect(result.current.startAttempt.status).toEqual('success'), { interval: 5 } @@ -46,6 +74,9 @@ describe('autostart', () => { it('does not start VNet if turned off', async () => { const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.isInitialized = true; + }); appContext.statePersistenceService.putState({ ...appContext.statePersistenceService.getState(), vnet: { autoStart: false }, @@ -60,6 +91,9 @@ describe('autostart', () => { it('switches off if start fails', async () => { const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.isInitialized = true; + }); const { statePersistenceService } = appContext; statePersistenceService.putState({ ...statePersistenceService.getState(), @@ -83,6 +117,9 @@ describe('autostart', () => { test('starting and stopping VNet toggles autostart', async () => { const appContext = new MockAppContext(); + appContext.workspacesService.setState(draft => { + draft.isInitialized = true; + }); const { statePersistenceService } = appContext; const { result } = renderHook(() => useVnetContext(), { wrapper: createWrapper(Wrapper, { appContext }), diff --git a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx index 12c856b737532..c8f75f22c59f3 100644 --- a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx +++ b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx @@ -30,6 +30,7 @@ import { useAsync, Attempt } from 'shared/hooks/useAsync'; import { useAppContext } from 'teleterm/ui/appContextProvider'; import { useAppState } from 'teleterm/ui/hooks/useAppState'; +import { useImmutableStore } from 'teleterm/ui/hooks/useImmutableStore'; /** * VnetContext manages the VNet instance. @@ -55,6 +56,10 @@ export const VnetContext = createContext(null); export const VnetContextProvider: FC = props => { const [status, setStatus] = useState('stopped'); const { vnet, mainProcessClient, configService } = useAppContext(); + const isWorkspaceStateInitialized = useImmutableStore( + 'workspacesService', + useCallback(state => state.isInitialized, []) + ); const [{ autoStart }, setAppState] = useAppState('vnet', { autoStart: false, }); @@ -88,7 +93,12 @@ export const VnetContextProvider: FC = props => { useEffect(() => { const handleAutoStart = async () => { - if (isSupported && autoStart && startAttempt.status === '') { + if ( + isSupported && + autoStart && + isWorkspaceStateInitialized && + startAttempt.status === '' + ) { const [, error] = await start(); // Turn off autostart if starting fails. Otherwise the user wouldn't be able to turn off @@ -100,7 +110,7 @@ export const VnetContextProvider: FC = props => { }; handleAutoStart(); - }, []); + }, [isWorkspaceStateInitialized]); return ( Date: Fri, 26 Apr 2024 11:17:53 +0200 Subject: [PATCH 8/9] Rename useAppState to usePersistedState --- .../teleterm/src/ui/Vnet/vnetContext.tsx | 4 ++-- ...ate.test.tsx => usePersistedState.test.tsx} | 17 ++++++++++------- .../{useAppState.ts => usePersistedState.ts} | 18 ++++++++++-------- .../statePersistenceService.ts | 6 +++--- 4 files changed, 25 insertions(+), 20 deletions(-) rename web/packages/teleterm/src/ui/hooks/{useAppState.test.tsx => usePersistedState.test.tsx} (85%) rename web/packages/teleterm/src/ui/hooks/{useAppState.ts => usePersistedState.ts} (80%) diff --git a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx index c8f75f22c59f3..46503503d9e9e 100644 --- a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx +++ b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx @@ -29,7 +29,7 @@ import { import { useAsync, Attempt } from 'shared/hooks/useAsync'; import { useAppContext } from 'teleterm/ui/appContextProvider'; -import { useAppState } from 'teleterm/ui/hooks/useAppState'; +import { usePersistedState } from 'teleterm/ui/hooks/usePersistedState'; import { useImmutableStore } from 'teleterm/ui/hooks/useImmutableStore'; /** @@ -60,7 +60,7 @@ export const VnetContextProvider: FC = props => { 'workspacesService', useCallback(state => state.isInitialized, []) ); - const [{ autoStart }, setAppState] = useAppState('vnet', { + const [{ autoStart }, setAppState] = usePersistedState('vnet', { autoStart: false, }); diff --git a/web/packages/teleterm/src/ui/hooks/useAppState.test.tsx b/web/packages/teleterm/src/ui/hooks/usePersistedState.test.tsx similarity index 85% rename from web/packages/teleterm/src/ui/hooks/useAppState.test.tsx rename to web/packages/teleterm/src/ui/hooks/usePersistedState.test.tsx index ddcb27da7d9a7..4aab64ddca95a 100644 --- a/web/packages/teleterm/src/ui/hooks/useAppState.test.tsx +++ b/web/packages/teleterm/src/ui/hooks/usePersistedState.test.tsx @@ -21,9 +21,9 @@ import { render, screen, act } from 'design/utils/testing'; import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; -import { useAppState } from './useAppState'; +import { usePersistedState } from './usePersistedState'; -it('propagates changes coming from the same useAppState invocation', () => { +it('propagates changes coming from the same usePersistedState invocation', () => { const appContext = new MockAppContext(); render( @@ -70,9 +70,9 @@ it('updates only the given key', () => { expect(appContext.statePersistenceService.getState()['foo']).toEqual('bar'); }); -// TODO(ravicious): Change the behavior of useAppState so it actually does propagate changes across -// callsites. -it('does not propagate changes across different useAppState invocations', () => { +// TODO(ravicious): Change the behavior of usePersistedState so it actually does propagate changes +// across callsites. +it('does not propagate changes across different usePersistedState invocations', () => { const appContext = new MockAppContext(); render( @@ -93,7 +93,10 @@ it('does not propagate changes across different useAppState invocations', () => type TestState = { counter: number; boolean: boolean }; const Counter = () => { - const [counter, setCounter] = useAppState<'counter', TestState>('counter', 0); + const [counter, setCounter] = usePersistedState<'counter', TestState>( + 'counter', + 0 + ); return (
@@ -104,7 +107,7 @@ const Counter = () => { }; const Boolean = () => { - const [boolean, setBoolean] = useAppState<'boolean', TestState>( + const [boolean, setBoolean] = usePersistedState<'boolean', TestState>( 'boolean', true ); diff --git a/web/packages/teleterm/src/ui/hooks/useAppState.ts b/web/packages/teleterm/src/ui/hooks/usePersistedState.ts similarity index 80% rename from web/packages/teleterm/src/ui/hooks/useAppState.ts rename to web/packages/teleterm/src/ui/hooks/usePersistedState.ts index 67eed048d2b73..51d9216881dc7 100644 --- a/web/packages/teleterm/src/ui/hooks/useAppState.ts +++ b/web/packages/teleterm/src/ui/hooks/usePersistedState.ts @@ -22,16 +22,17 @@ import { useAppContext } from 'teleterm/ui/appContextProvider'; import { StatePersistenceState } from 'teleterm/ui/services/statePersistence'; /** - * useAppState is like useState, but it persists the state to app_state.json under the given key. + * usePersistedState is like useState, but it persists the state to app_state.json under the given + * key. * - * IMPORTANT: Currently, useAppState doesn't propagate changes across several callsites. That is, if - * two callsites use the same key, calling setState in one component will not cause the other + * IMPORTANT: Currently, usePersistedState doesn't propagate changes across several callsites. That + * is, if two callsites use the same key, calling setState in one component will not cause the other * component to update. * * This will _not_ work as expected: * * const Counter = () => { - * const [count, setCount] = useAppState('count', 0); + * const [count, setCount] = usePersistedState('count', 0); * * return ( *
@@ -63,7 +64,7 @@ import { StatePersistenceState } from 'teleterm/ui/services/statePersistence'; * } * * () => { - * const [count, setCount] = useAppState('count', 0); + * const [count, setCount] = usePersistedState('count', 0); * * return ( * <> @@ -73,7 +74,7 @@ import { StatePersistenceState } from 'teleterm/ui/services/statePersistence'; * ); * } */ -export function useAppState< +export function usePersistedState< // key could've been any string, but in lieu of avoiding typos, it's better to take it // from one central definition. Key extends keyof WholeState, @@ -86,9 +87,10 @@ export function useAppState< const { statePersistenceService } = useAppContext(); const wholeState = statePersistenceService.getState() as WholeState; const state = Object.hasOwn(wholeState, key) ? wholeState[key] : initialState; - // TODO(ravicious): useAppState currently doesn't propagate changes across several callsites. + // TODO(ravicious): usePersistedState currently doesn't propagate changes across several + // callsites. // - // useAppState should either use useSyncExternalStore or some other solution to register a + // usePersistedState should either use useSyncExternalStore or some other solution to register a // listener in statePersistenceService that gets called whenever the given key gets updated. const [, rerender] = useState(); diff --git a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts index cb88456eca51a..ba37afef48ecf 100644 --- a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts +++ b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts @@ -46,7 +46,7 @@ export interface StatePersistenceState { vnet: { autoStart: boolean }; } -// Before adding new methods to this service, consider using useAppState instead. +// Before adding new methods to this service, consider using usePersistedState instead. export class StatePersistenceService { constructor(private _fileStorage: FileStorage) {} @@ -100,8 +100,8 @@ export class StatePersistenceService { getState(): StatePersistenceState { // Some legacy callsites expected StatePersistenceService to manage the default state for them, - // but with the move towards useAppState, we should put the default state close to where it's - // going to be used. Hence the use of Partial here. + // but with the move towards usePersistedState, we should put the default state close to where + // it's going to be used. Hence the use of Partial here. const defaultState: Partial = { connectionTracker: { connections: [], From 0d64323c6aa07a738e61ed3c2bbec34a34463374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 26 Apr 2024 11:29:24 +0200 Subject: [PATCH 9/9] Rename useImmutableStore to useStoreSelector --- web/packages/teleterm/src/ui/Vnet/vnetContext.tsx | 4 ++-- .../{useImmutableStore.ts => useStoreSelector.ts} | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) rename web/packages/teleterm/src/ui/hooks/{useImmutableStore.ts => useStoreSelector.ts} (74%) diff --git a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx index 46503503d9e9e..d7a3b71605f16 100644 --- a/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx +++ b/web/packages/teleterm/src/ui/Vnet/vnetContext.tsx @@ -30,7 +30,7 @@ import { useAsync, Attempt } from 'shared/hooks/useAsync'; import { useAppContext } from 'teleterm/ui/appContextProvider'; import { usePersistedState } from 'teleterm/ui/hooks/usePersistedState'; -import { useImmutableStore } from 'teleterm/ui/hooks/useImmutableStore'; +import { useStoreSelector } from 'teleterm/ui/hooks/useStoreSelector'; /** * VnetContext manages the VNet instance. @@ -56,7 +56,7 @@ export const VnetContext = createContext(null); export const VnetContextProvider: FC = props => { const [status, setStatus] = useState('stopped'); const { vnet, mainProcessClient, configService } = useAppContext(); - const isWorkspaceStateInitialized = useImmutableStore( + const isWorkspaceStateInitialized = useStoreSelector( 'workspacesService', useCallback(state => state.isInitialized, []) ); diff --git a/web/packages/teleterm/src/ui/hooks/useImmutableStore.ts b/web/packages/teleterm/src/ui/hooks/useStoreSelector.ts similarity index 74% rename from web/packages/teleterm/src/ui/hooks/useImmutableStore.ts rename to web/packages/teleterm/src/ui/hooks/useStoreSelector.ts index ac24cfeb007f0..f0f2f07cbece2 100644 --- a/web/packages/teleterm/src/ui/hooks/useImmutableStore.ts +++ b/web/packages/teleterm/src/ui/hooks/useStoreSelector.ts @@ -23,11 +23,16 @@ import { IAppContext } from 'teleterm/ui/types'; import { ImmutableStore } from 'teleterm/ui/services/immutableStore'; /** - * useImmutableStore selects a value out of a store and triggers component update whenever that - * value changes. The selector needs to have stable identity. + * useStoreSelector selects a value out of a store and triggers a component update whenever that + * value changes. + * + * The selector needs to have stable identity, i.e., the selector needs to return a piece of the + * store state instead of creating a new object or an array on each invocation. For example, if you + * need two separate pieces from the same store, call useStoreSelector twice with different + * selectors instead of combining two pieces of state into a new object. * * @example - * const isInitialized = useImmutableStore( + * const isInitialized = useStoreSelector( * 'workspacesService', * useCallback(state => state.isInitialized, []) * ); @@ -38,10 +43,10 @@ import { ImmutableStore } from 'teleterm/ui/services/immutableStore'; * * // Defined inside a React component. * () => { - * const isInitialized = useImmutableStore('workspacesService', getIsInitialized); + * const isInitialized = useStoreSelector('workspacesService', getIsInitialized); * } */ -export const useImmutableStore = < +export const useStoreSelector = < SelectedState, StoreKey extends ImmutableStoreKeys, >(