⚡️ (#2462): remove memory leak from unnecessary listeners

Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>

package-lock.json

@@ -56,6 +56,7 @@
 				"markdown-it": "^13.0.0",
 				"markdown-it-container": "^3.0.0",
 				"markdown-it-task-lists": "^2.1.1",
+				"mitt": "^3.0.0",
 				"prosemirror-collab": "^1.2.2",
 				"prosemirror-inputrules": "^1.1.3",
 				"prosemirror-markdown": "^1.8.0",
@@ -13806,6 +13807,11 @@
 				"node": ">=0.10.0"
 			}
 		},
+		"node_modules/mitt": {
+			"version": "3.0.0",
+			"resolved": "https://registry.npmjs.org/mitt/-/mitt-3.0.0.tgz",
+			"integrity": "sha512-7dX2/10ITVyqh4aOSVI9gdape+t9l2/8QxHrFmUXu4EEUpdlxl6RudZUPZoc+zuY2hk1j7XxVroIVIan/pD/SQ=="
+		},
 		"node_modules/mkdirp": {
 			"version": "0.5.5",
 			"resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz",
@@ -29512,6 +29518,11 @@
 				}
 			}
 		},
+		"mitt": {
+			"version": "3.0.0",
+			"resolved": "https://registry.npmjs.org/mitt/-/mitt-3.0.0.tgz",
+			"integrity": "sha512-7dX2/10ITVyqh4aOSVI9gdape+t9l2/8QxHrFmUXu4EEUpdlxl6RudZUPZoc+zuY2hk1j7XxVroIVIan/pD/SQ=="
+		},
 		"mkdirp": {
 			"version": "0.5.5",
 			"resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz",

package.json

@@ -65,14 +65,15 @@
 		"@tiptap/extension-underline": "^2.0.0-beta.23",
 		"@tiptap/suggestion": "^2.0.0-beta.92",
 		"@tiptap/vue-2": "^2.0.0-beta.78",
-		"debounce": "^1.2.1",
 		"core-js": "^3.22.7",
+		"debounce": "^1.2.1",
 		"escape-html": "^1.0.3",
 		"highlight.js": "^10.7.2",
 		"lowlight": "^1.20.0",
 		"markdown-it": "^13.0.0",
 		"markdown-it-container": "^3.0.0",
 		"markdown-it-task-lists": "^2.1.1",
+		"mitt": "^3.0.0",
 		"prosemirror-collab": "^1.2.2",
 		"prosemirror-inputrules": "^1.1.3",
 		"prosemirror-markdown": "^1.8.0",

src/components/EditorWrapper.vue

@@ -343,30 +343,20 @@ export default {
 		this.close()
 	},
 	methods: {
-		async close() {
-			clearInterval(this.saveStatusPolling)
-			if (this.currentSession && this.$syncService) {
-				try {
-					await this.$syncService.close()
-					this.currentSession = null
-					this.$syncService = null
-				} catch (e) {
-					// Ignore issues closing the session since those might happen due to network issues
-				}
-			}
-			return true
-		},
 		updateLastSavedStatus() {
 			if (this.document) {
 				this.lastSavedString = moment(this.document.lastSavedVersionTime * 1000).fromNow()
 			}
 		},
+
 		initSession() {
 			if (!this.hasDocumentParameters) {
 				this.$parent.$emit('error', 'No valid file provided')
 				return
 			}
+
 			const guestName = localStorage.getItem('nick') ? localStorage.getItem('nick') : getRandomGuestName()
+
 			this.$syncService = new SyncService({
 				guestName,
 				shareToken: this.shareToken,
@@ -380,156 +370,9 @@ export default {
 
 				},
 			})
-				.on('opened', ({ document, session }) => {
-					this.currentSession = session
-					this.document = document
-					this.readOnly = document.readOnly
-					this.lock = this.$syncService.lock
-					localStorage.setItem('nick', this.currentSession.guestName)
-					this.$store.dispatch('setCurrentSession', this.currentSession)
-				})
-				.on('change', ({ document, sessions }) => {
-					if (this.document.baseVersionEtag !== '' && document.baseVersionEtag !== this.document.baseVersionEtag) {
-						this.resolveUseServerVersion()
-						return
-					}
-					this.updateSessions.bind(this)(sessions)
-					this.document = document
 
-					this.syncError = null
+			this.listenSyncServiceEvents()
-					this.$editor.setOptions({ editable: !this.readOnly })
+
-				})
-				.on('loaded', ({ documentSource }) => {
-					this.hasConnectionIssue = false
-					const content = this.isRichEditor
-						? markdownit.render(documentSource)
-						: '<pre>' + escapeHtml(documentSource) + '</pre>'
-					const language = extensionHighlight[this.fileExtension] || this.fileExtension
-					loadSyntaxHighlight(language).then(() => {
-						this.$editor = createEditor({
-							content,
-							onCreate: ({ editor }) => {
-								this.$syncService.state = editor.state
-								this.$syncService.startSync()
-							},
-							onUpdate: ({ editor }) => {
-								this.$syncService.state = editor.state
-							},
-							extensions: [
-								Collaboration.configure({
-									// the initial version we start with
-									// version is an integer which is incremented with every change
-									version: this.document.initialVersion,
-									clientID: this.currentSession.id,
-									// debounce changes so we can save some bandwidth
-									debounce: EDITOR_PUSH_DEBOUNCE,
-									onSendable: ({ sendable }) => {
-										if (this.$syncService) {
-											this.$syncService.sendSteps()
-										}
-									},
-									update: ({ steps, version, editor }) => {
-										const { state, view, schema } = editor
-										if (getVersion(state) > version) {
-											return
-										}
-										const tr = receiveTransaction(
-											state,
-											steps.map(item => Step.fromJSON(schema, item.step)),
-											steps.map(item => item.clientID),
-										)
-										tr.setMeta('clientID', steps.map(item => item.clientID))
-										view.dispatch(tr)
-									},
-								}),
-								Keymap.configure({
-									'Mod-s': () => {
-										this.$syncService.save()
-										return true
-									},
-								}),
-								UserColor.configure({
-									clientID: this.currentSession.id,
-									color: (clientID) => {
-										const session = this.sessions.find(item => '' + item.id === '' + clientID)
-										return session?.color
-									},
-									name: (clientID) => {
-										const session = this.sessions.find(item => '' + item.id === '' + clientID)
-										return session?.userId ? session.userId : session?.guestName
-									},
-								}),
-							],
-							enableRichEditing: this.isRichEditor,
-							currentDirectory: this.currentDirectory,
-						})
-						this.$editor.on('focus', () => {
-							this.$emit('focus')
-						})
-						this.$editor.on('blur', () => {
-							this.$emit('blur')
-						})
-						this.$syncService.state = this.$editor.state
-					})
-				})
-				.on('sync', ({ steps, document }) => {
-					this.hasConnectionIssue = false
-					try {
-						const collaboration = this.$editor.extensionManager.extensions.find(e => e.name === 'collaboration')
-						collaboration.options.update({
-							version: document.currentVersion,
-							steps,
-							editor: this.$editor,
-						})
-						this.$syncService.state = this.$editor.state
-						this.updateLastSavedStatus()
-					} catch (e) {
-						console.error('Failed to update steps in collaboration plugin', e)
-						// TODO: we should recreate the editing session when this happens
-					}
-					this.document = document
-				})
-				.on('error', (error, data) => {
-					this.$editor.setOptions({ editable: false })
-					if (error === ERROR_TYPE.SAVE_COLLISSION && (!this.syncError || this.syncError.type !== ERROR_TYPE.SAVE_COLLISSION)) {
-						this.contentLoaded = true
-						this.syncError = {
-							type: error,
-							data,
-						}
-					}
-					if (error === ERROR_TYPE.CONNECTION_FAILED && !this.hasConnectionIssue) {
-						this.hasConnectionIssue = true
-						// FIXME: ideally we just try to reconnect in the service, so we don't loose steps
-						OC.Notification.showTemporary('Connection failed, reconnecting')
-						if (data.retry !== false) {
-							setTimeout(this.reconnect.bind(this), 5000)
-						}
-					}
-					if (error === ERROR_TYPE.SOURCE_NOT_FOUND) {
-						this.hasConnectionIssue = true
-					}
-					this.$emit('ready')
-				})
-				.on('stateChange', (state) => {
-					if (state.initialLoading && !this.contentLoaded) {
-						this.contentLoaded = true
-						if (this.autofocus && !this.readOnly) {
-							this.$editor.commands.focus()
-						}
-						this.$emit('ready')
-						this.$parent.$emit('ready', true)
-					}
-					if (Object.prototype.hasOwnProperty.call(state, 'dirty')) {
-						this.dirty = state.dirty
-					}
-				})
-				.on('idle', () => {
-					this.$syncService.close()
-					this.idle = true
-					this.readOnly = true
-					this.$editor.setOptions({ editable: !this.readOnly })
-				})
 			this.$syncService.open({
 				fileId: this.fileId,
 				filePath: this.relativePath,
@@ -540,6 +383,28 @@ export default {
 			this.forceRecreate = false
 		},
 
+		listenSyncServiceEvents() {
+			this.$syncService
+				.on('opened', this.onOpened)
+				.on('change', this.onChange)
+				.on('loaded', this.onLoaded)
+				.on('sync', this.onSync)
+				.on('error', this.onError)
+				.on('stateChange', this.onStateChange)
+				.on('idle', this.onIdle)
+		},
+
+		unlistenSyncServiceEvents() {
+			this.$syncService
+				.off('opened', this.onOpened)
+				.off('change', this.onChange)
+				.off('loaded', this.onLoaded)
+				.off('sync', this.onSync)
+				.off('error', this.onError)
+				.off('stateChange', this.onStateChange)
+				.off('idle', this.onIdle)
+		},
+
 		resolveUseThisVersion() {
 			this.$syncService.forceSave()
 			this.$editor.setOptions({ editable: !this.readOnly })
@@ -553,19 +418,25 @@ export default {
 		reconnect() {
 			this.contentLoaded = false
 			this.hasConnectionIssue = false
-			if (this.$syncService) {
+
-				this.$syncService.close().then(() => {
+			const connect = () => {
-					this.$syncService = null
+				this.unlistenSyncServiceEvents()
-					this.$editor.destroy()
-					this.initSession()
-				}).catch((e) => {
-					// Ignore issues closing the session since those might happen due to network issues
-				})
-			} else {
 				this.$syncService = null
 				this.$editor.destroy()
 				this.initSession()
 			}
+
+			if (this.$syncService) {
+				this.$syncService
+					.close()
+					.then(connect)
+					.catch((e) => {
+					// Ignore issues closing the session since those might happen due to network issues
+					})
+			} else {
+				connect()
+			}
+
 			this.idle = false
 		},
 
@@ -602,6 +473,181 @@ export default {
 				}
 			}
 		},
+
+		onOpened({ document, session }) {
+			this.currentSession = session
+			this.document = document
+			this.readOnly = document.readOnly
+			this.lock = this.$syncService.lock
+			localStorage.setItem('nick', this.currentSession.guestName)
+			this.$store.dispatch('setCurrentSession', this.currentSession)
+		},
+
+		onLoaded({ documentSource }) {
+			this.hasConnectionIssue = false
+			const content = this.isRichEditor
+				? markdownit.render(documentSource)
+				: '<pre>' + escapeHtml(documentSource) + '</pre>'
+			const language = extensionHighlight[this.fileExtension] || this.fileExtension
+
+			loadSyntaxHighlight(language)
+				.then(() => {
+					this.$editor = createEditor({
+						content,
+						onCreate: ({ editor }) => {
+							this.$syncService.state = editor.state
+							this.$syncService.startSync()
+						},
+						onUpdate: ({ editor }) => {
+							this.$syncService.state = editor.state
+						},
+						extensions: [
+							Collaboration.configure({
+								// the initial version we start with
+								// version is an integer which is incremented with every change
+								version: this.document.initialVersion,
+								clientID: this.currentSession.id,
+								// debounce changes so we can save some bandwidth
+								debounce: EDITOR_PUSH_DEBOUNCE,
+								onSendable: ({ sendable }) => {
+									if (this.$syncService) {
+										this.$syncService.sendSteps()
+									}
+								},
+								update: ({ steps, version, editor }) => {
+									const { state, view, schema } = editor
+									if (getVersion(state) > version) {
+										return
+									}
+									const tr = receiveTransaction(
+										state,
+										steps.map(item => Step.fromJSON(schema, item.step)),
+										steps.map(item => item.clientID),
+									)
+									tr.setMeta('clientID', steps.map(item => item.clientID))
+									view.dispatch(tr)
+								},
+							}),
+							Keymap.configure({
+								'Mod-s': () => {
+									this.$syncService.save()
+									return true
+								},
+							}),
+							UserColor.configure({
+								clientID: this.currentSession.id,
+								color: (clientID) => {
+									const session = this.sessions.find(item => '' + item.id === '' + clientID)
+									return session?.color
+								},
+								name: (clientID) => {
+									const session = this.sessions.find(item => '' + item.id === '' + clientID)
+									return session?.userId ? session.userId : session?.guestName
+								},
+							}),
+						],
+						enableRichEditing: this.isRichEditor,
+						currentDirectory: this.currentDirectory,
+					})
+					this.$editor.on('focus', () => {
+						this.$emit('focus')
+					})
+					this.$editor.on('blur', () => {
+						this.$emit('blur')
+					})
+					this.$syncService.state = this.$editor.state
+				})
+
+		},
+
+		onChange({ document, sessions }) {
+			if (this.document.baseVersionEtag !== '' && document.baseVersionEtag !== this.document.baseVersionEtag) {
+				this.resolveUseServerVersion()
+				return
+			}
+			this.updateSessions.bind(this)(sessions)
+			this.document = document
+
+			this.syncError = null
+			this.$editor.setOptions({ editable: !this.readOnly })
+		},
+
+		onSync({ steps, document }) {
+			this.hasConnectionIssue = false
+			try {
+				const collaboration = this.$editor.extensionManager.extensions.find(e => e.name === 'collaboration')
+				collaboration.options.update({
+					version: document.currentVersion,
+					steps,
+					editor: this.$editor,
+				})
+				this.$syncService.state = this.$editor.state
+				this.updateLastSavedStatus()
+			} catch (e) {
+				console.error('Failed to update steps in collaboration plugin', e)
+				// TODO: we should recreate the editing session when this happens
+			}
+			this.document = document
+		},
+
+		onError({ type, data }) {
+			this.$editor.setOptions({ editable: false })
+			if (type === ERROR_TYPE.SAVE_COLLISSION && (!this.syncError || this.syncError.type !== ERROR_TYPE.SAVE_COLLISSION)) {
+				this.contentLoaded = true
+				this.syncError = {
+					type,
+					data,
+				}
+			}
+			if (type === ERROR_TYPE.CONNECTION_FAILED && !this.hasConnectionIssue) {
+				this.hasConnectionIssue = true
+				// FIXME: ideally we just try to reconnect in the service, so we don't loose steps
+				OC.Notification.showTemporary('Connection failed, reconnecting')
+				if (data.retry !== false) {
+					setTimeout(this.reconnect.bind(this), 5000)
+				}
+			}
+			if (type === ERROR_TYPE.SOURCE_NOT_FOUND) {
+				this.hasConnectionIssue = true
+			}
+			this.$emit('ready')
+		},
+
+		onStateChange(state) {
+			if (state.initialLoading && !this.contentLoaded) {
+				this.contentLoaded = true
+				if (this.autofocus && !this.readOnly) {
+					this.$editor.commands.focus()
+				}
+				this.$emit('ready')
+				this.$parent.$emit('ready', true)
+			}
+			if (Object.prototype.hasOwnProperty.call(state, 'dirty')) {
+				this.dirty = state.dirty
+			}
+		},
+
+		onIdle() {
+			this.$syncService.close()
+			this.idle = true
+			this.readOnly = true
+			this.$editor.setOptions({ editable: !this.readOnly })
+		},
+
+		async close() {
+			clearInterval(this.saveStatusPolling)
+			if (this.currentSession && this.$syncService) {
+				try {
+					await this.$syncService.close()
+					this.unlistenSyncServiceEvents()
+					this.currentSession = null
+					this.$syncService = null
+				} catch (e) {
+					// Ignore issues closing the session since those might happen due to network issues
+				}
+			}
+			return true
+		},
 	},
 }
 </script>

src/services/PollingBackend.js

@@ -176,7 +176,7 @@ class PollingBackend {
 		if (!e.response || e.code === 'ECONNABORTED') {
 			if (this.fetchRetryCounter++ >= MAX_RETRY_FETCH_COUNT) {
 				console.error('[PollingBackend:fetchSteps] Network error when fetching steps, emitting CONNECTION_FAILED')
-				this._authority.emit('error', ERROR_TYPE.CONNECTION_FAILED, { retry: false })
+				this._authority.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: { retry: false } })
 
 			} else {
 				console.error(`[PollingBackend:fetchSteps] Network error when fetching steps, retry ${this.fetchRetryCounter}`)
@@ -184,22 +184,25 @@ class PollingBackend {
 		} else if (e.response.status === 409 && e.response.data.document.currentVersion === this._authority.document.currentVersion) {
 			// Only emit conflict event if we have synced until the latest version
 			console.error('Conflict during file save, please resolve')
-			this._authority.emit('error', ERROR_TYPE.SAVE_COLLISSION, {
+			this._authority.emit('error', {
-				outsideChange: e.response.data.outsideChange,
+				type: ERROR_TYPE.SAVE_COLLISSION,
+				data: {
+					outsideChange: e.response.data.outsideChange,
+				},
 			})
 		} else if (e.response.status === 403) {
-			this._authority.emit('error', ERROR_TYPE.SOURCE_NOT_FOUND, {})
+			this._authority.emit('error', { type: ERROR_TYPE.SOURCE_NOT_FOUND, data: {} })
 			this.disconnect()
 		} else if (e.response.status === 404) {
-			this._authority.emit('error', ERROR_TYPE.SOURCE_NOT_FOUND, {})
+			this._authority.emit('error', { type: ERROR_TYPE.SOURCE_NOT_FOUND, data: {} })
 			this.disconnect()
 		} else if (e.response.status === 503) {
 			this.increaseRefetchTimer()
-			this._authority.emit('error', ERROR_TYPE.CONNECTION_FAILED, { retry: false })
+			this._authority.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: { retry: false } })
 			console.error('Failed to fetch steps due to unavailable service', e)
 		} else {
 			this.disconnect()
-			this._authority.emit('error', ERROR_TYPE.CONNECTION_FAILED, { retry: false })
+			this._authority.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: { retry: false } })
 			console.error('Failed to fetch steps due to other reason', e)
 		}
 
@@ -232,7 +235,7 @@ class PollingBackend {
 			console.error('failed to apply steps due to collission, retrying')
 			this.lock = false
 			if (!response || code === 'ECONNABORTED') {
-				this._authority.emit('error', ERROR_TYPE.CONNECTION_FAILED, {})
+				this._authority.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} })
 				return
 			}
 			const { status, data } = response
@@ -243,7 +246,7 @@ class PollingBackend {
 				}
 				// Only emit conflict event if we have synced until the latest version
 				if (data.document?.currentVersion === this._authority.document.currentVersion) {
-					this._authority.emit('error', ERROR_TYPE.PUSH_FAILURE, {})
+					this._authority.emit('error', { type: ERROR_TYPE.PUSH_FAILURE, data: {} })
 					OC.Notification.showTemporary('Changes could not be sent yet')
 				}
 			}
@@ -304,7 +307,7 @@ class PollingBackend {
 		const newRetry = this.retryTime ? Math.min(this.retryTime * 2, MAX_PUSH_RETRY) : MIN_PUSH_RETRY
 		if (newRetry > WARNING_PUSH_RETRY && this.retryTime < WARNING_PUSH_RETRY) {
 			OC.Notification.showTemporary('Changes could not be sent yet')
-			this._authority.emit('error', ERROR_TYPE.PUSH_FAILURE, {})
+			this._authority.emit('error', { type: ERROR_TYPE.PUSH_FAILURE, data: {} })
 		}
 		this.retryTime = newRetry
 	}

src/services/SyncService.d.ts

@@ -0,0 +1,26 @@
+export declare type EventTypes = {
+  /* Document state */
+  opened: unknown;
+  loaded: unknown;
+
+  /* All initial steps fetched */
+  fetched: unknown;
+
+  /* received new steps */
+  sync: unknown;
+
+  /* state changed (dirty) */
+  stateChange: unknown;
+
+  /* error */
+  error: unknown;
+
+  /* Events for session and document meta data */
+  change: unknown;
+
+  /* Emitted after successful save */
+  save: unknown;
+
+  /* Emitted once a document becomes idle */
+  idle: unknown;
+};

src/services/SyncService.js

@@ -1,3 +1,4 @@
+/* eslint-disable jsdoc/valid-types */
 /*
  * @copyright Copyright (c) 2019 Julius Härtl <jus@bitgrid.net>
  *
@@ -24,6 +25,7 @@ import axios from '@nextcloud/axios'
 import PollingBackend from './PollingBackend.js'
 import { endpointUrl } from './../helpers/index.js'
 import { getVersion, sendableSteps } from 'prosemirror-collab'
+import mitt from 'mitt'
 
 const defaultOptions = {
 	shareToken: null,
@@ -60,25 +62,8 @@ const ERROR_TYPE = {
 class SyncService {
 
 	constructor(options) {
-		this.eventHandlers = {
+		/** @type {import('mitt').Emitter<import('./SyncService').EventTypes>} _bus */
-			/* Document state */
+		this._bus = mitt()
-			opened: [],
-			loaded: [],
-			/* All initial steps fetched */
-			fetched: [],
-			/* received new steps */
-			sync: [],
-			/* state changed (dirty) */
-			stateChange: [],
-			/* error */
-			error: [],
-			/* Events for session and document meta data */
-			change: [],
-			/* Emitted after successful save */
-			save: [],
-			/* Emitted once a document becomes idle */
-			idle: [],
-		}
 
 		this.backend = new PollingBackend(this)
 
@@ -132,9 +117,9 @@ class SyncService {
 		})
 			.then(response => response.data, error => {
 				if (!error.response || error.code === 'ECONNABORTED') {
-					this.emit('error', ERROR_TYPE.CONNECTION_FAILED, {})
+					this.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} })
 				} else {
-					this.emit('error', ERROR_TYPE.LOAD_ERROR, error.response.status)
+					this.emit('error', { type: ERROR_TYPE.LOAD_ERROR, data: error.response.status })
 				}
 				throw error
 			})
@@ -322,19 +307,18 @@ class SyncService {
 		return axios.post(url, params)
 	}
 
-	on(event, callback, _this) {
+	on(event, callback) {
-		this.eventHandlers[event].push(callback.bind(_this))
+		this._bus.on(event, callback)
 		return this
 	}
 
-	emit(event, data, additionalData) {
+	off(event, callback) {
-		if (typeof this.eventHandlers[event] !== 'undefined') {
+		this._bus.off(event, callback)
-			this.eventHandlers[event].forEach(function(callback) {
+		return this
-				callback(data, additionalData)
+	}
-			})
+
-		} else {
+	emit(event, data) {
-			console.error('Event not found', event)
+		this._bus.emit(event, data)
-		}
 	}
 
 	isPublic() {