From bfa0459e723a6a8200290c60099bbb2d239a2302 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Thu, 10 Jul 2025 08:53:47 +0100 Subject: [PATCH] Merge pull request #26928 from overleaf/td-remove-use-scope-value Remove useScopeValue and its associated store GitOrigin-RevId: 439d6eb16343f65695ef615a9ff697d0cc5ad2c7 --- .../ide-react/context/ide-react-context.tsx | 30 +----------- .../js/shared/context/ide-context.tsx | 7 +-- .../hooks/use-scope-value-setter-only.ts | 38 --------------- .../js/shared/hooks/use-scope-value.ts | 46 ------------------- .../web/frontend/stories/decorators/scope.tsx | 31 +------------ .../source-editor/helpers/mock-scope.ts | 3 -- .../frontend/helpers/editor-providers.tsx | 11 +---- 7 files changed, 5 insertions(+), 161 deletions(-) delete mode 100644 services/web/frontend/js/shared/hooks/use-scope-value-setter-only.ts delete mode 100644 services/web/frontend/js/shared/hooks/use-scope-value.ts diff --git a/services/web/frontend/js/features/ide-react/context/ide-react-context.tsx b/services/web/frontend/js/features/ide-react/context/ide-react-context.tsx index a55fc3236b..cad238f53c 100644 --- a/services/web/frontend/js/features/ide-react/context/ide-react-context.tsx +++ b/services/web/frontend/js/features/ide-react/context/ide-react-context.tsx @@ -42,33 +42,8 @@ export const IdeReactContext = createContext( undefined ) -function populateIdeReactScope(store: ReactScopeValueStore) { - store.set('settings', {}) -} - -function populatePdfScope(store: ReactScopeValueStore) { - store.allowNonExistentPath('pdf', true) -} - -export function createReactScopeValueStore() { - const scopeStore = new ReactScopeValueStore() - - // Populate the scope value store with default values that will be used by - // nested contexts that refer to scope values. The ideal would be to leave - // initialization of store values up to the nested context, which would keep - // initialization code together with the context and would only populate - // necessary values in the store, but this is simpler for now - populateIdeReactScope(scopeStore) - populatePdfScope(scopeStore) - - scopeStore.allowNonExistentPath('hasLintingError') - - return scopeStore -} - export const IdeReactProvider: FC = ({ children }) => { const projectId = getMeta('ol-project_id') - const [scopeStore] = useState(() => createReactScopeValueStore()) const [eventEmitter] = useState(createIdeEventEmitter) const [permissionsLevel, setPermissionsLevel] = useState('readOnly') @@ -146,8 +121,6 @@ export const IdeReactProvider: FC = ({ children }) => { joinProject(project as unknown as ProjectMetadata) setPermissionsLevel(permissionsLevel) - // Make watchers update immediately - scopeStore.flushUpdates() eventEmitter.emit('project:joined', { project, permissionsLevel }) setProjectJoined(true) } @@ -157,7 +130,7 @@ export const IdeReactProvider: FC = ({ children }) => { return () => { socket.removeListener('joinProjectResponse', handleJoinProjectResponse) } - }, [socket, eventEmitter, scopeStore, joinProject]) + }, [socket, eventEmitter, joinProject]) const ide = useMemo(() => { return { @@ -194,7 +167,6 @@ export const IdeReactProvider: FC = ({ children }) => { diff --git a/services/web/frontend/js/shared/context/ide-context.tsx b/services/web/frontend/js/shared/context/ide-context.tsx index 899dcbce5b..f882680583 100644 --- a/services/web/frontend/js/shared/context/ide-context.tsx +++ b/services/web/frontend/js/shared/context/ide-context.tsx @@ -8,7 +8,6 @@ export type Ide = { } type IdeContextValue = Ide & { - scopeStore: ScopeValueStore scopeEventEmitter: ScopeEventEmitter unstableStore: ScopeValueStore } @@ -18,11 +17,10 @@ export const IdeContext = createContext(undefined) export const IdeProvider: FC< React.PropsWithChildren<{ ide: Ide - scopeStore: ScopeValueStore scopeEventEmitter: ScopeEventEmitter unstableStore: ScopeValueStore }> -> = ({ ide, scopeStore, scopeEventEmitter, unstableStore, children }) => { +> = ({ ide, scopeEventEmitter, unstableStore, children }) => { /** * Expose unstableStore via `window.overleaf.unstable.store`, so it can be accessed by external extensions. * @@ -49,11 +47,10 @@ export const IdeProvider: FC< const value = useMemo(() => { return { ...ide, - scopeStore, scopeEventEmitter, unstableStore, } - }, [ide, scopeStore, scopeEventEmitter, unstableStore]) + }, [ide, scopeEventEmitter, unstableStore]) return {children} } diff --git a/services/web/frontend/js/shared/hooks/use-scope-value-setter-only.ts b/services/web/frontend/js/shared/hooks/use-scope-value-setter-only.ts deleted file mode 100644 index 27564a1d6a..0000000000 --- a/services/web/frontend/js/shared/hooks/use-scope-value-setter-only.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { - type Dispatch, - type SetStateAction, - useCallback, - useState, -} from 'react' -import { useIdeContext } from '../context/ide-context' -import _ from 'lodash' - -/** - * Similar to `useScopeValue`, but instead of creating a two-way binding, only - * changes in react-> angular direction are propagated, with `value` remaining - * local and independent of its value in the Angular scope. - * - * The interface is compatible with React.useState(), including - * the option of passing a function to the setter. - */ -export default function useScopeValueSetterOnly( - path: string, // dot '.' path of a property in the Angular scope. - defaultValue?: T -): [T | undefined, Dispatch>] { - const { scopeStore } = useIdeContext() - - const [value, setValue] = useState(defaultValue) - - const scopeSetter = useCallback( - (newValue: SetStateAction) => { - setValue(val => { - const actualNewValue = _.isFunction(newValue) ? newValue(val) : newValue - scopeStore.set(path, actualNewValue) - return actualNewValue - }) - }, - [path, scopeStore] - ) - - return [value, scopeSetter] -} diff --git a/services/web/frontend/js/shared/hooks/use-scope-value.ts b/services/web/frontend/js/shared/hooks/use-scope-value.ts deleted file mode 100644 index 594a71f92a..0000000000 --- a/services/web/frontend/js/shared/hooks/use-scope-value.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { - type Dispatch, - type SetStateAction, - useCallback, - useEffect, - useState, -} from 'react' -import _ from 'lodash' -import { useIdeContext } from '../context/ide-context' - -/** - * Binds a property in an Angular scope making it accessible in a React - * component. The interface is compatible with React.useState(), including - * the option of passing a function to the setter. - * - * The generic type is not an actual guarantee because the value for a path is - * returned as undefined when there is nothing in the scope store for that path. - */ -export default function useScopeValue( - path: string // dot '.' path of a property in the Angular scope -): [T, Dispatch>] { - const { scopeStore } = useIdeContext() - - const [value, setValue] = useState(() => scopeStore.get(path)) - - useEffect(() => { - return scopeStore.watch(path, (newValue: T) => { - // NOTE: this is deliberately wrapped in a function, - // to avoid calling setValue directly with a value that's a function - setValue(() => newValue) - }) - }, [path, scopeStore]) - - const scopeSetter = useCallback( - (newValue: SetStateAction) => { - setValue(val => { - const actualNewValue = _.isFunction(newValue) ? newValue(val) : newValue - scopeStore.set(path, actualNewValue) - return actualNewValue - }) - }, - [path, scopeStore] - ) - - return [value, scopeSetter] -} diff --git a/services/web/frontend/stories/decorators/scope.tsx b/services/web/frontend/stories/decorators/scope.tsx index 8977f582ab..9bde1eebfa 100644 --- a/services/web/frontend/stories/decorators/scope.tsx +++ b/services/web/frontend/stories/decorators/scope.tsx @@ -10,10 +10,7 @@ import useFetchMock from '../hooks/use-fetch-mock' import { useMeta } from '../hooks/use-meta' import SocketIOShim, { SocketIOMock } from '@/ide/connection/SocketIoShim' import { IdeContext } from '@/shared/context/ide-context' -import { - IdeReactContext, - createReactScopeValueStore, -} from '@/features/ide-react/context/ide-react-context' +import { IdeReactContext } from '@/features/ide-react/context/ide-react-context' import { IdeEventEmitter } from '@/features/ide-react/create-ide-event-emitter' import { ReactScopeValueStore } from '@/features/ide-react/scope-value-store/react-scope-value-store' import { ReactScopeEventEmitter } from '@/features/ide-react/scope-event-emitter/react-scope-event-emitter' @@ -57,27 +54,6 @@ const project: Project = { ], } -const initialScope = { - user, - project, - settings: { - pdfViewer: 'js', - syntaxValidation: true, - }, - editor: { - richText: false, - - // FIXME: This is pretty useless because the editor relies on a much more fleshed-out document, so we rely on overriding it in individual stories - sharejs_doc: { - doc_id: 'test-doc', - getSnapshot: () => 'some doc content', - hasBufferedOps: () => false, - }, - open_doc_name: 'testfile.tex', - }, - permissionsLevel: 'owner', -} - const socket = new SocketIOShim.SocketShimNoop( new SocketIOMock() ) as unknown as Socket @@ -195,10 +171,6 @@ const IdeReactProvider: FC = ({ children }) => { })) const [ideContextValue] = useState(() => { - const scopeStore = createReactScopeValueStore() - for (const [key, value] of Object.entries(initialScope)) { - scopeStore.set(key, value) - } const scopeEventEmitter = new ReactScopeEventEmitter(new IdeEventEmitter()) const unstableStore = new ReactScopeValueStore() @@ -212,7 +184,6 @@ const IdeReactProvider: FC = ({ children }) => { return { socket, - scopeStore, scopeEventEmitter, unstableStore, } diff --git a/services/web/test/frontend/features/source-editor/helpers/mock-scope.ts b/services/web/test/frontend/features/source-editor/helpers/mock-scope.ts index 830a9c97a2..fd19ce4f85 100644 --- a/services/web/test/frontend/features/source-editor/helpers/mock-scope.ts +++ b/services/web/test/frontend/features/source-editor/helpers/mock-scope.ts @@ -27,8 +27,5 @@ export const mockScope = ( refreshResolvedCommentsDropdown: cy.stub(() => sleep(1000)), onlineUserCursorHighlights: {}, permissionsLevel: 'owner', - $on: cy.stub().log(false), - $broadcast: cy.stub().log(false), - $emit: cy.stub().log(false), } } diff --git a/services/web/test/frontend/helpers/editor-providers.tsx b/services/web/test/frontend/helpers/editor-providers.tsx index a82adefdbd..accf5ad73d 100644 --- a/services/web/test/frontend/helpers/editor-providers.tsx +++ b/services/web/test/frontend/helpers/editor-providers.tsx @@ -11,10 +11,7 @@ import React, { type FC, type PropsWithChildren, } from 'react' -import { - createReactScopeValueStore, - IdeReactContext, -} from '@/features/ide-react/context/ide-react-context' +import { IdeReactContext } from '@/features/ide-react/context/ide-react-context' import { IdeEventEmitter } from '@/features/ide-react/create-ide-event-emitter' import { ReactScopeValueStore } from '@/features/ide-react/scope-value-store/react-scope-value-store' import { ReactScopeEventEmitter } from '@/features/ide-react/scope-event-emitter/react-scope-event-emitter' @@ -300,11 +297,6 @@ const makeIdeReactProvider = ( })) const [ideContextValue] = useState(() => { - const scopeStore = createReactScopeValueStore() - for (const [key, value] of Object.entries(scope)) { - // TODO: path for nested entries - scopeStore.set(key, value) - } const scopeEventEmitter = new ReactScopeEventEmitter( new IdeEventEmitter() ) @@ -312,7 +304,6 @@ const makeIdeReactProvider = ( return { socket, - scopeStore, scopeEventEmitter, unstableStore, }