From 1fcf94c3b9d7a4aa6d44b667a520526683319756 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Mon, 14 Dec 2020 12:44:10 +0100 Subject: [PATCH] Merge pull request #3436 from overleaf/msm-react-shared-context React shared context GitOrigin-RevId: ebc6fa90dd8c65ddf803fd457c99a30f0e8e3c9c --- services/web/app/views/project/editor.pug | 2 + .../chat/controllers/chat-controller.js | 6 +- .../features/chat/store/chat-store-effect.js | 65 ++++++++++--------- .../js/features/chat/store/chat-store.js | 21 ++++-- .../outline/components/outline-pane.js | 5 +- .../outline/controllers/outline-controller.js | 13 +++- services/web/frontend/js/ide.js | 8 +++ .../js/shared/context/application-context.js | 27 ++++++++ .../js/shared/context/editor-context.js | 27 ++++++++ .../js/shared/context/root-context.js | 15 +++++ .../web/frontend/stories/outline.stories.js | 13 +++- services/web/package-lock.json | 17 ++++- services/web/package.json | 1 + .../chat/components/chat-pane.test.js | 28 +++++--- .../features/chat/components/stubs.js | 2 - .../features/chat/store/chat-store.test.js | 22 +++++-- .../outline/components/outline-pane.test.js | 11 ++-- .../frontend/helpers/render-with-context.js | 14 ++++ 18 files changed, 232 insertions(+), 65 deletions(-) create mode 100644 services/web/frontend/js/shared/context/application-context.js create mode 100644 services/web/frontend/js/shared/context/editor-context.js create mode 100644 services/web/frontend/js/shared/context/root-context.js create mode 100644 services/web/test/frontend/helpers/render-with-context.js diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index 28242837f2..6612ca8f90 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -7,6 +7,8 @@ block vars block content .editor(ng-controller="IdeController").full-size + //- required by react2angular-shared-context, must be rendered as a top level component + shared-context-react() .loading-screen(ng-if="state.loading") .loading-screen-brand-container .loading-screen-brand( diff --git a/services/web/frontend/js/features/chat/controllers/chat-controller.js b/services/web/frontend/js/features/chat/controllers/chat-controller.js index 4170ce3c5e..7e6046f14e 100644 --- a/services/web/frontend/js/features/chat/controllers/chat-controller.js +++ b/services/web/frontend/js/features/chat/controllers/chat-controller.js @@ -1,5 +1,6 @@ import App from '../../../base' import { react2angular } from 'react2angular' +import { rootContext } from '../../../shared/context/root-context' import ChatPane from '../components/chat-pane' @@ -8,4 +9,7 @@ App.controller('ReactChatController', function($scope, ide) { ide.$scope.$broadcast('chat:resetUnreadMessages') }) -App.component('chat', react2angular(ChatPane)) +App.component( + 'chat', + react2angular(rootContext.use(ChatPane), ['resetUnreadMessages']) +) diff --git a/services/web/frontend/js/features/chat/store/chat-store-effect.js b/services/web/frontend/js/features/chat/store/chat-store-effect.js index 3a03391999..8066bdf991 100644 --- a/services/web/frontend/js/features/chat/store/chat-store-effect.js +++ b/services/web/frontend/js/features/chat/store/chat-store-effect.js @@ -1,37 +1,38 @@ -import { useState, useEffect } from 'react' +import { useState, useEffect, useRef } from 'react' import { ChatStore } from './chat-store' - -let chatStore - -export function resetChatStore() { - chatStore = undefined -} +import { useApplicationContext } from '../../../shared/context/application-context' +import { useEditorContext } from '../../../shared/context/editor-context' export function useChatStore() { - if (!chatStore) { - chatStore = new ChatStore() + const { user } = useApplicationContext() + const { projectId } = useEditorContext() + + const chatStoreRef = useRef(new ChatStore(user, projectId)) + + const [atEnd, setAtEnd] = useState(chatStoreRef.current.atEnd) + const [loading, setLoading] = useState(chatStoreRef.current.loading) + const [messages, setMessages] = useState(chatStoreRef.current.messages) + + useEffect( + () => { + const chatStore = chatStoreRef.current + function handleStoreUpdated() { + setAtEnd(chatStore.atEnd) + setLoading(chatStore.loading) + setMessages(chatStore.messages) + } + chatStore.on('updated', handleStoreUpdated) + return () => chatStore.destroy() + }, + [chatStoreRef] + ) + + return { + userId: user.id, + atEnd, + loading, + messages, + loadMoreMessages: () => chatStoreRef.current.loadMoreMessages(), + sendMessage: message => chatStoreRef.current.sendMessage(message) } - - function getStateFromStore() { - return { - userId: window.user.id, - atEnd: chatStore.atEnd, - loading: chatStore.loading, - messages: chatStore.messages, - loadMoreMessages: () => chatStore.loadMoreMessages(), - sendMessage: message => chatStore.sendMessage(message) - } - } - - const [storeState, setStoreState] = useState(getStateFromStore()) - - useEffect(() => { - function handleStoreUpdated() { - setStoreState(getStateFromStore()) - } - chatStore.on('updated', handleStoreUpdated) - return () => chatStore.off('updated', handleStoreUpdated) - }, []) - - return storeState } diff --git a/services/web/frontend/js/features/chat/store/chat-store.js b/services/web/frontend/js/features/chat/store/chat-store.js index c20538da23..beba1e34f3 100644 --- a/services/web/frontend/js/features/chat/store/chat-store.js +++ b/services/web/frontend/js/features/chat/store/chat-store.js @@ -5,19 +5,21 @@ import { getJSON, postJSON } from '../../../infrastructure/fetch-json' export const MESSAGE_LIMIT = 50 export class ChatStore { - constructor() { + constructor(user, projectId) { this.messages = [] this.loading = false this.atEnd = false + this._user = user + this._projectId = projectId this._nextBeforeTimestamp = null this._justSent = false this._emitter = new EventEmitter() - window._ide.socket.on('new-chat-message', message => { + this._onNewChatMessage = message => { const messageIsFromSelf = - message && message.user && message.user.id === window.user.id + message && message.user && message.user.id === this._user.id if (!messageIsFromSelf || !this._justSent) { this.messages = appendMessage(this.messages, message) this._emitter.emit('updated') @@ -27,7 +29,14 @@ export class ChatStore { ) } this._justSent = false - }) + } + + window._ide.socket.on('new-chat-message', this._onNewChatMessage) + } + + destroy() { + window._ide.socket.off('new-chat-message', this._onNewChatMessage) + this._emitter.off() // removes all listeners } on(event, fn) { @@ -76,11 +85,11 @@ export class ChatStore { } this._justSent = true this.messages = appendMessage(this.messages, { - user: window.user, + user: this._user, content: message, timestamp: Date.now() }) - const url = `/project/${window.project_id}/messages` + const url = `/project/${this._projectId}/messages` this._emitter.emit('updated') return postJSON(url, { body }) } diff --git a/services/web/frontend/js/features/outline/components/outline-pane.js b/services/web/frontend/js/features/outline/components/outline-pane.js index bb3d91013b..b19d4ffc9b 100644 --- a/services/web/frontend/js/features/outline/components/outline-pane.js +++ b/services/web/frontend/js/features/outline/components/outline-pane.js @@ -7,11 +7,11 @@ import OutlineRoot from './outline-root' import Icon from '../../../shared/components/icon' import localStorage from '../../../infrastructure/local-storage' import withErrorBoundary from '../../../infrastructure/error-boundary' +import { useEditorContext } from '../../../shared/context/editor-context' function OutlinePane({ isTexFile, outline, - projectId, jumpToLine, onToggle, eventTracking, @@ -19,6 +19,8 @@ function OutlinePane({ }) { const { t } = useTranslation() + const { projectId } = useEditorContext() + const storageKey = `file_outline.expanded.${projectId}` const [expanded, setExpanded] = useState(() => { const storedExpandedState = localStorage.getItem(storageKey) !== false @@ -77,7 +79,6 @@ function OutlinePane({ OutlinePane.propTypes = { isTexFile: PropTypes.bool.isRequired, outline: PropTypes.array.isRequired, - projectId: PropTypes.string.isRequired, jumpToLine: PropTypes.func.isRequired, onToggle: PropTypes.func.isRequired, eventTracking: PropTypes.object.isRequired, diff --git a/services/web/frontend/js/features/outline/controllers/outline-controller.js b/services/web/frontend/js/features/outline/controllers/outline-controller.js index e1836e1187..da5a2125ea 100644 --- a/services/web/frontend/js/features/outline/controllers/outline-controller.js +++ b/services/web/frontend/js/features/outline/controllers/outline-controller.js @@ -1,6 +1,7 @@ import App from '../../../base' import OutlinePane from '../components/outline-pane' import { react2angular } from 'react2angular' +import { rootContext } from '../../../shared/context/root-context' App.controller('OutlineController', function($scope, ide, eventTracking) { $scope.isTexFile = false @@ -30,4 +31,14 @@ App.controller('OutlineController', function($scope, ide, eventTracking) { }) // Wrap React component as Angular component. Only needed for "top-level" component -App.component('outlinePane', react2angular(OutlinePane)) +App.component( + 'outlinePane', + react2angular(rootContext.use(OutlinePane), [ + 'outline', + 'jumpToLine', + 'highlightedLine', + 'eventTracking', + 'onToggle', + 'isTexFile' + ]) +) diff --git a/services/web/frontend/js/ide.js b/services/web/frontend/js/ide.js index 6333276d21..fc7da903f8 100644 --- a/services/web/frontend/js/ide.js +++ b/services/web/frontend/js/ide.js @@ -61,6 +61,10 @@ import './main/account-upgrade' import './main/exposed-settings' import './main/system-messages' import '../../modules/modules-ide.js' + +import { react2angular } from 'react2angular' +import { rootContext } from './shared/context/root-context' + App.controller('IdeController', function( $scope, $timeout, @@ -349,6 +353,10 @@ If the project has been renamed please look in your project list for a new proje }) }) +// required by react2angular-shared-context, maps the shared context instance to an angular component +// that must be rendered in the app +App.component('sharedContextReact', react2angular(rootContext.component)) + export default angular.bootstrap(document.body, ['SharelatexApp']) function __guard__(value, transform) { diff --git a/services/web/frontend/js/shared/context/application-context.js b/services/web/frontend/js/shared/context/application-context.js new file mode 100644 index 0000000000..3668ac1037 --- /dev/null +++ b/services/web/frontend/js/shared/context/application-context.js @@ -0,0 +1,27 @@ +import React, { createContext, useContext } from 'react' +import PropTypes from 'prop-types' + +export const ApplicationContext = createContext() + +export function ApplicationProvider({ children }) { + return ( + + {children} + + ) +} + +ApplicationProvider.propTypes = { + children: PropTypes.any +} + +export function useApplicationContext() { + const { user } = useContext(ApplicationContext) + return { + user + } +} diff --git a/services/web/frontend/js/shared/context/editor-context.js b/services/web/frontend/js/shared/context/editor-context.js new file mode 100644 index 0000000000..49e272cd99 --- /dev/null +++ b/services/web/frontend/js/shared/context/editor-context.js @@ -0,0 +1,27 @@ +import React, { createContext, useContext } from 'react' +import PropTypes from 'prop-types' + +export const EditorContext = createContext() + +export function EditorProvider({ children }) { + return ( + + {children} + + ) +} + +EditorProvider.propTypes = { + children: PropTypes.any +} + +export function useEditorContext() { + const { projectId } = useContext(EditorContext) + return { + projectId + } +} diff --git a/services/web/frontend/js/shared/context/root-context.js b/services/web/frontend/js/shared/context/root-context.js new file mode 100644 index 0000000000..f06186f972 --- /dev/null +++ b/services/web/frontend/js/shared/context/root-context.js @@ -0,0 +1,15 @@ +import React from 'react' +import { ApplicationProvider } from './application-context' +import { EditorProvider } from './editor-context' +import createSharedContext from 'react2angular-shared-context' + +// eslint-disable-next-line react/prop-types +export function ContextRoot({ children }) { + return ( + + {children} + + ) +} + +export const rootContext = createSharedContext(ContextRoot) diff --git a/services/web/frontend/stories/outline.stories.js b/services/web/frontend/stories/outline.stories.js index a70652d4e0..8035915a69 100644 --- a/services/web/frontend/stories/outline.stories.js +++ b/services/web/frontend/stories/outline.stories.js @@ -1,6 +1,9 @@ import React from 'react' import OutlinePane from '../js/features/outline/components/outline-pane' +import { ContextRoot } from '../js/shared/context/root-context' + +window.project_id = '1234' export const Basic = args => Basic.args = { @@ -47,11 +50,17 @@ export default { jumpToLine: { action: 'jumpToLine' } }, args: { - projectId: '1234', eventTracking: { sendMB: () => {} }, isTexFile: true, outline: [], jumpToLine: () => {}, onToggle: () => {} - } + }, + decorators: [ + Story => ( + + + + ) + ] } diff --git a/services/web/package-lock.json b/services/web/package-lock.json index 46db6fd3f7..2daca71265 100644 --- a/services/web/package-lock.json +++ b/services/web/package-lock.json @@ -23924,7 +23924,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": false, "integrity": "sha512-SknJC52obPfGQPnjIkXbmA6+5H15E+fR+E4iR2oQ3zzCLbd7/ONua69R/Gw7AgkTLsRG+r5fzksYwWe1AgTyWA==", "requires": { "minimist": "0.0.8" @@ -30003,6 +30003,21 @@ "ngcomponent": "^4.1.0" } }, + "react2angular-shared-context": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/react2angular-shared-context/-/react2angular-shared-context-1.1.0.tgz", + "integrity": "sha512-8es9/tSf0XxbJ6f/58OPVrNUKjzY370WJXhMo5e4klxaEW0obLK2MRidIT8aY1ZZAf/x9EAGlBFJlvrxhu0H0Q==", + "requires": { + "uuid": "^7.0.3" + }, + "dependencies": { + "uuid": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/uuid/-/uuid-7.0.3.tgz", + "integrity": "sha512-DPSke0pXhTZgoF/d+WSt2QaKMCFSfx7QegxEWT+JOuHF5aWrKEn0G+ztjuJg/gG8/ItK+rbPCD/yNv8yyih6Cg==" + } + } + }, "reactcss": { "version": "1.2.3", "resolved": "https://registry.npmjs.org/reactcss/-/reactcss-1.2.3.tgz", diff --git a/services/web/package.json b/services/web/package.json index 9c7625476c..0c8efd24aa 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -133,6 +133,7 @@ "react-i18next": "^11.7.1", "react-linkify": "^1.0.0-alpha", "react2angular": "^4.0.6", + "react2angular-shared-context": "^1.1.0", "request": "^2.88.2", "request-promise-native": "^1.0.8", "requestretry": "^1.13.0", diff --git a/services/web/test/frontend/features/chat/components/chat-pane.test.js b/services/web/test/frontend/features/chat/components/chat-pane.test.js index dd4cc51fc3..150a20fe07 100644 --- a/services/web/test/frontend/features/chat/components/chat-pane.test.js +++ b/services/web/test/frontend/features/chat/components/chat-pane.test.js @@ -1,13 +1,10 @@ import React from 'react' import { expect } from 'chai' -import { - render, - screen, - waitForElementToBeRemoved -} from '@testing-library/react' +import { screen, waitForElementToBeRemoved } from '@testing-library/react' import fetchMock from 'fetch-mock' import ChatPane from '../../../../../frontend/js/features/chat/components/chat-pane' +import { renderWithEditorContext } from '../../../helpers/render-with-context' import { stubChatStore, stubMathJax, @@ -53,31 +50,44 @@ describe('', function() { it('renders multiple messages', async function() { fetchMock.get(/messages/, testMessages) - render( {}} />) + // unmounting before `beforeEach` block is executed is required to prevent cleanup errors + const { unmount } = renderWithEditorContext( + {}} /> + ) await screen.findByText('a message') await screen.findByText('another message') + unmount() }) it('A loading spinner is rendered while the messages are loading, then disappears', async function() { fetchMock.get(/messages/, []) - render( {}} />) + const { unmount } = renderWithEditorContext( + {}} /> + ) await waitForElementToBeRemoved(() => screen.getByText('Loading…')) + unmount() }) describe('"send your first message" placeholder', function() { it('is rendered when there are no messages ', async function() { fetchMock.get(/messages/, []) - render( {}} />) + const { unmount } = renderWithEditorContext( + {}} /> + ) await screen.findByText('Send your first message to your collaborators') + unmount() }) it('is not rendered when messages are displayed', function() { fetchMock.get(/messages/, testMessages) - render( {}} />) + const { unmount } = renderWithEditorContext( + {}} /> + ) expect( screen.queryByText('Send your first message to your collaborators') ).to.not.exist + unmount() }) }) }) diff --git a/services/web/test/frontend/features/chat/components/stubs.js b/services/web/test/frontend/features/chat/components/stubs.js index 4c7d5f30db..4faed6ac2f 100644 --- a/services/web/test/frontend/features/chat/components/stubs.js +++ b/services/web/test/frontend/features/chat/components/stubs.js @@ -1,5 +1,4 @@ import sinon from 'sinon' -import { resetChatStore } from '../../../../../frontend/js/features/chat/store/chat-store-effect' export function stubUIConfig() { window.uiConfig = { @@ -30,7 +29,6 @@ export function tearDownMathJaxStubs() { export function stubChatStore({ user }) { window._ide = { socket: { on: sinon.stub(), off: sinon.stub() } } window.user = user - resetChatStore() } export function tearDownChatStore() { diff --git a/services/web/test/frontend/features/chat/store/chat-store.test.js b/services/web/test/frontend/features/chat/store/chat-store.test.js index a505c98ddc..a4226f1fbe 100644 --- a/services/web/test/frontend/features/chat/store/chat-store.test.js +++ b/services/web/test/frontend/features/chat/store/chat-store.test.js @@ -13,6 +13,8 @@ describe('ChatStore', function() { id: '123abc' } + const testProjectId = 'project-123' + const testMessage = { content: 'hello', timestamp: new Date().getTime(), @@ -22,15 +24,13 @@ describe('ChatStore', function() { beforeEach(function() { fetchMock.reset() - window.user = user - window.project_id = 'project-123' window.csrfToken = 'csrf_tok' - socket = { on: sinon.stub() } + socket = { on: sinon.stub(), off: sinon.stub() } window._ide = { socket } mockSocketMessage = message => socket.on.getCall(0).args[1](message) - store = new ChatStore() + store = new ChatStore(user, testProjectId) }) afterEach(function() { @@ -216,4 +216,18 @@ describe('ChatStore', function() { expect(subscriber).not.to.be.called }) }) + + describe('destroy', function() { + beforeEach(function() { + fetchMock.post(/messages/, 204) + }) + + it('removes event listeners', async function() { + const subscriber = sinon.stub() + store.on('updated', subscriber) + store.destroy() + await store.sendMessage('a message') + expect(subscriber).not.to.be.called + }) + }) }) diff --git a/services/web/test/frontend/features/outline/components/outline-pane.test.js b/services/web/test/frontend/features/outline/components/outline-pane.test.js index a4798e12aa..1f54ecfbf7 100644 --- a/services/web/test/frontend/features/outline/components/outline-pane.test.js +++ b/services/web/test/frontend/features/outline/components/outline-pane.test.js @@ -1,16 +1,20 @@ import { expect } from 'chai' import React from 'react' import sinon from 'sinon' -import { screen, render, fireEvent } from '@testing-library/react' +import { screen, fireEvent } from '@testing-library/react' import OutlinePane from '../../../../../frontend/js/features/outline/components/outline-pane' +import { renderWithEditorContext } from '../../../helpers/render-with-context' describe('', function() { const jumpToLine = () => {} - const projectId = '123abc' const onToggle = sinon.stub() const eventTracking = { sendMB: sinon.stub() } + function render(children) { + renderWithEditorContext(children, { projectId: '123abc' }) + } + before(function() { global.localStorage = { getItem: sinon.stub().returns(null), @@ -41,7 +45,6 @@ describe('', function() { ', function() { ', function() { + {children} + + ) +}