Merge pull request #3436 from overleaf/msm-react-shared-context

React shared context

GitOrigin-RevId: ebc6fa90dd8c65ddf803fd457c99a30f0e8e3c9c

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(

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'])
+)

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
 }

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 })
   }

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,

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'
+  ])
+)

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) {

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 (
+    <ApplicationContext.Provider
+      value={{
+        user: window.user
+      }}
+    >
+      {children}
+    </ApplicationContext.Provider>
+  )
+}
+
+ApplicationProvider.propTypes = {
+  children: PropTypes.any
+}
+
+export function useApplicationContext() {
+  const { user } = useContext(ApplicationContext)
+  return {
+    user
+  }
+}

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 (
+    <EditorContext.Provider
+      value={{
+        projectId: window.project_id
+      }}
+    >
+      {children}
+    </EditorContext.Provider>
+  )
+}
+
+EditorProvider.propTypes = {
+  children: PropTypes.any
+}
+
+export function useEditorContext() {
+  const { projectId } = useContext(EditorContext)
+  return {
+    projectId
+  }
+}

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 (
+    <ApplicationProvider>
+      <EditorProvider>{children}</EditorProvider>
+    </ApplicationProvider>
+  )
+}
+
+export const rootContext = createSharedContext(ContextRoot)

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 => <OutlinePane {...args} />
 Basic.args = {
@@ -47,11 +50,17 @@ export default {
     jumpToLine: { action: 'jumpToLine' }
   },
   args: {
-    projectId: '1234',
     eventTracking: { sendMB: () => {} },
     isTexFile: true,
     outline: [],
     jumpToLine: () => {},
     onToggle: () => {}
-  }
+  },
+  decorators: [
+    Story => (
+      <ContextRoot>
+        <Story />
+      </ContextRoot>
+    )
+  ]
 }

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",

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",

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('<ChatPane />', function() {
 
   it('renders multiple messages', async function() {
     fetchMock.get(/messages/, testMessages)
-    render(<ChatPane resetUnreadMessages={() => {}} />)
+    // unmounting before `beforeEach` block is executed is required to prevent cleanup errors
+    const { unmount } = renderWithEditorContext(
+      <ChatPane resetUnreadMessages={() => {}} />
+    )
 
     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(<ChatPane resetUnreadMessages={() => {}} />)
+    const { unmount } = renderWithEditorContext(
+      <ChatPane resetUnreadMessages={() => {}} />
+    )
     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(<ChatPane resetUnreadMessages={() => {}} />)
+      const { unmount } = renderWithEditorContext(
+        <ChatPane resetUnreadMessages={() => {}} />
+      )
       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(<ChatPane resetUnreadMessages={() => {}} />)
+      const { unmount } = renderWithEditorContext(
+        <ChatPane resetUnreadMessages={() => {}} />
+      )
       expect(
         screen.queryByText('Send your first message to your collaborators')
       ).to.not.exist
+      unmount()
     })
   })
 })

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() {

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
+    })
+  })
 })

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('<OutlinePane />', 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('<OutlinePane />', function() {
       <OutlinePane
         isTexFile
         outline={outline}
-        projectId={projectId}
         jumpToLine={jumpToLine}
         onToggle={onToggle}
         eventTracking={eventTracking}
@@ -57,7 +60,6 @@ describe('<OutlinePane />', function() {
       <OutlinePane
         isTexFile={false}
         outline={outline}
-        projectId={projectId}
         jumpToLine={jumpToLine}
         onToggle={onToggle}
         eventTracking={eventTracking}
@@ -80,7 +82,6 @@ describe('<OutlinePane />', function() {
       <OutlinePane
         isTexFile
         outline={outline}
-        projectId={projectId}
         jumpToLine={jumpToLine}
         onToggle={onToggle}
         eventTracking={eventTracking}

services/web/test/frontend/helpers/render-with-context.js

@@ -0,0 +1,14 @@
+import React from 'react'
+import { render } from '@testing-library/react'
+import { ApplicationProvider } from '../../../frontend/js/shared/context/application-context'
+import { EditorProvider } from '../../../frontend/js/shared/context/editor-context'
+
+export function renderWithEditorContext(children, { user, projectId } = {}) {
+  window.user = user || window.user
+  window.project_id = projectId != null ? projectId : window.project_id
+  return render(
+    <ApplicationProvider>
+      <EditorProvider>{children}</EditorProvider>
+    </ApplicationProvider>
+  )
+}