DEV: Use @bind instead of repeated .bind(this) (#14931)

Fixes some cases where event listeners weren't correctly removed. Also fixes a dependency tracking bug in user-private-messages

app/assets/javascripts/discourse/app/components/bookmark.js

@@ -17,7 +17,7 @@ import { TIME_SHORTCUT_TYPES } from "discourse/lib/time-shortcut";
 import { action } from "@ember/object";
 import { ajax } from "discourse/lib/ajax";
 import bootbox from "bootbox";
-import discourseComputed, { on } from "discourse-common/utils/decorators";
+import discourseComputed, { bind, on } from "discourse-common/utils/decorators";
 import { formattedReminderTime } from "discourse/lib/bookmark";
 import { and, notEmpty } from "@ember/object/computed";
 import { popupAjaxError } from "discourse/lib/ajax-error";
@@ -65,7 +65,7 @@ export default Component.extend({
       _itsatrap: new ItsATrap(),
     });
 
-    this.registerOnCloseHandler(this._onModalClose.bind(this));
+    this.registerOnCloseHandler(this._onModalClose);
 
     this._loadBookmarkOptions();
     this._bindKeyboardShortcuts();
@@ -239,6 +239,7 @@ export default Component.extend({
     }
   },
 
+  @bind
   _onModalClose(closeOpts) {
     // we want to close without saving if the user already saved
     // manually or deleted the bookmark, as well as when the modal

app/assets/javascripts/discourse/app/components/d-editor.js

@@ -319,7 +319,7 @@ export default Component.extend(TextareaTextManipulation, {
     }
 
     if (isTesting()) {
-      this.element.addEventListener("paste", this.paste.bind(this));
+      this.element.addEventListener("paste", this.paste);
     }
   },
 

app/assets/javascripts/discourse/app/components/reviewable-item.js

@@ -4,7 +4,7 @@ import I18n from "I18n";
 import { ajax } from "discourse/lib/ajax";
 import bootbox from "bootbox";
 import { dasherize } from "@ember/string";
-import discourseComputed from "discourse-common/utils/decorators";
+import discourseComputed, { bind } from "discourse-common/utils/decorators";
 import optionalService from "discourse/lib/optional-service";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import { set } from "@ember/object";
@@ -113,6 +113,7 @@ export default Component.extend({
     return _components[type];
   },
 
+  @bind
   _performConfirmed(action) {
     let reviewable = this.reviewable;
 
@@ -264,7 +265,7 @@ export default Component.extend({
           title: "review.reject_reason.title",
           model: this.reviewable,
         }).setProperties({
-          performConfirmed: this._performConfirmed.bind(this),
+          performConfirmed: this._performConfirmed,
           action,
         });
       } else if (customModal) {
@@ -272,7 +273,7 @@ export default Component.extend({
           title: `review.${customModal}.title`,
           model: this.reviewable,
         }).setProperties({
-          performConfirmed: this._performConfirmed.bind(this),
+          performConfirmed: this._performConfirmed,
           action,
         });
       } else {

app/assets/javascripts/discourse/app/controllers/user-private-messages.js

@@ -27,12 +27,20 @@ export default Controller.extend({
     return pmView === VIEW_NAME_WARNINGS && !viewingSelf && !isAdmin;
   },
 
-  @discourseComputed("pmTopicTrackingState.newIncoming.[]", "group")
+  @discourseComputed(
+    "pmTopicTrackingState.newIncoming.[]",
+    "pmTopicTrackingState.statesModificationCounter",
+    "group"
+  )
   newLinkText() {
     return this._linkText("new");
   },
 
-  @discourseComputed("pmTopicTrackingState.newIncoming.[]", "group")
+  @discourseComputed(
+    "pmTopicTrackingState.newIncoming.[]",
+    "pmTopicTrackingState.statesModificationCounter",
+    "group"
+  )
   unreadLinkText() {
     return this._linkText("unread");
   },

app/assets/javascripts/discourse/app/lib/uppy-checksum-plugin.js

@@ -1,6 +1,7 @@
 import { UploadPreProcessorPlugin } from "discourse/lib/uppy-plugin-base";
 import { Promise } from "rsvp";
 import { HUGE_FILE_THRESHOLD_BYTES } from "discourse/mixins/uppy-upload";
+import { bind } from "discourse-common/utils/decorators";
 
 export default class UppyChecksum extends UploadPreProcessorPlugin {
   static pluginId = "uppy-checksum";
@@ -33,6 +34,7 @@ export default class UppyChecksum extends UploadPreProcessorPlugin {
     return true;
   }
 
+  @bind
   _generateChecksum(fileIds) {
     if (!this._canUseSubtleCrypto()) {
       return this._skipAll(fileIds, true);
@@ -85,14 +87,14 @@ export default class UppyChecksum extends UploadPreProcessorPlugin {
   }
 
   _hasCryptoCipher() {
-    return window.crypto && window.crypto.subtle && window.crypto.subtle.digest;
+    return window.crypto?.subtle?.digest;
   }
 
   install() {
-    this._install(this._generateChecksum.bind(this));
+    this._install(this._generateChecksum);
   }
 
   uninstall() {
-    this._uninstall(this._generateChecksum.bind(this));
+    this._uninstall(this._generateChecksum);
   }
 }

app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js

@@ -1,5 +1,6 @@
 import { UploadPreProcessorPlugin } from "discourse/lib/uppy-plugin-base";
 import { Promise } from "rsvp";
+import { bind } from "discourse-common/utils/decorators";
 
 export default class UppyMediaOptimization extends UploadPreProcessorPlugin {
   static pluginId = "uppy-media-optimization";
@@ -15,6 +16,7 @@ export default class UppyMediaOptimization extends UploadPreProcessorPlugin {
     this.runParallel = opts.runParallel || false;
   }
 
+  @bind
   _optimizeFile(fileId) {
     let file = this._getFile(fileId);
 
@@ -42,13 +44,15 @@ export default class UppyMediaOptimization extends UploadPreProcessorPlugin {
       });
   }
 
+  @bind
   _optimizeParallel(fileIds) {
-    return Promise.all(fileIds.map(this._optimizeFile.bind(this)));
+    return Promise.all(fileIds.map(this._optimizeFile));
   }
 
+  @bind
   async _optimizeSerial(fileIds) {
     let optimizeTasks = fileIds.map((fileId) => () =>
-      this._optimizeFile.call(this, fileId)
+      this._optimizeFile(fileId)
     );
 
     for (const task of optimizeTasks) {
@@ -58,17 +62,17 @@ export default class UppyMediaOptimization extends UploadPreProcessorPlugin {
 
   install() {
     if (this.runParallel) {
-      this._install(this._optimizeParallel.bind(this));
+      this._install(this._optimizeParallel);
     } else {
-      this._install(this._optimizeSerial.bind(this));
+      this._install(this._optimizeSerial);
     }
   }
 
   uninstall() {
     if (this.runParallel) {
-      this._uninstall(this._optimizeParallel.bind(this));
+      this._uninstall(this._optimizeParallel);
     } else {
-      this._uninstall(this._optimizeSerial.bind(this));
+      this._uninstall(this._optimizeSerial);
     }
   }
 }

app/assets/javascripts/discourse/app/mixins/card-contents-base.js

@@ -98,14 +98,14 @@ export default Mixin.create({
 
   didInsertElement() {
     this._super(...arguments);
-    afterTransition($(this.element), this._hide.bind(this));
+    afterTransition($(this.element), this._hide);
     const id = this.elementId;
     const triggeringLinkClass = this.triggeringLinkClass;
     const previewClickEvent = `click.discourse-preview-${id}-${triggeringLinkClass}`;
     const mobileScrollEvent = "scroll.mobile-card-cloak";
 
     this.setProperties({
-      boundCardClickHandler: this._cardClickHandler.bind(this),
+      boundCardClickHandler: this._cardClickHandler,
       previewClickEvent,
       mobileScrollEvent,
     });
@@ -127,6 +127,7 @@ export default Mixin.create({
     );
   },
 
+  @bind
   _cardClickHandler(event) {
     if (this.avatarSelector) {
       let matched = this._showCardOnClick(
@@ -290,6 +291,7 @@ export default Mixin.create({
     return mainOutletOffset.top - outletHeights;
   },
 
+  @bind
   _hide() {
     if (!this.visible) {
       $(this.element).css({ left: -9999, top: -9999 });

app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js

@@ -10,7 +10,7 @@ import { warn } from "@ember/debug";
 import I18n from "I18n";
 import getURL from "discourse-common/lib/get-url";
 import { clipboardHelpers } from "discourse/lib/utilities";
-import { observes, on } from "discourse-common/utils/decorators";
+import { bind, observes, on } from "discourse-common/utils/decorators";
 import {
   bindFileInputChangeListener,
   displayErrorForUpload,
@@ -33,6 +33,8 @@ import bootbox from "bootbox";
 // functionality and event binding.
 //
 export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
+  uploadTargetBound: false,
+
   @observes("composerModel.uploadCancelled")
   _cancelUpload() {
     if (!this.get("composerModel.uploadCancelled")) {
@@ -46,17 +48,18 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
 
   @on("willDestroyElement")
   _unbindUploadTarget() {
+    if (!this.uploadTargetBound) {
+      return;
+    }
+
     this.fileInputEl?.removeEventListener(
       "change",
       this.fileInputEventListener
     );
 
-    this.element?.removeEventListener("paste", this.pasteEventListener);
+    this.element.removeEventListener("paste", this.pasteEventListener);
 
-    this.appEvents.off(
+    this.appEvents.off(`${this.eventPrefix}:add-files`, this._addFiles);
-      `${this.eventPrefix}:add-files`,
-      this._addFiles.bind(this)
-    );
 
     this._reset();
 
@@ -64,6 +67,8 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
       this._uppyInstance.close();
       this._uppyInstance = null;
     }
+
+    this.uploadTargetBound = false;
   },
 
   _abortAndReset() {
@@ -79,14 +84,14 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
     this.fileInputEl = document.getElementById(this.fileUploadElementId);
     const isPrivateMessage = this.get("composerModel.privateMessage");
 
-    this.appEvents.on(
+    this.appEvents.on(`${this.eventPrefix}:add-files`, this._addFiles);
-      `${this.eventPrefix}:add-files`,
-      this._addFiles.bind(this)
-    );
 
     this._unbindUploadTarget();
-    this._bindFileInputChangeListener();
+    this.fileInputEventListener = bindFileInputChangeListener(
-    this._bindPasteListener();
+      this.fileInputEl,
+      this._addFiles
+    );
+    this.element.addEventListener("paste", this.pasteEventListener);
 
     this._uppyInstance = new Uppy({
       id: this.uppyId,
@@ -222,7 +227,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
       );
     });
 
-    this._uppyInstance.on("upload-error", this._handleUploadError.bind(this));
+    this._uppyInstance.on("upload-error", this._handleUploadError);
 
     this._uppyInstance.on("complete", () => {
       this.appEvents.trigger(`${this.eventPrefix}:all-uploads-complete`);
@@ -250,8 +255,11 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
 
     this._setupPreProcessors();
     this._setupUIPlugins();
+
+    this.uploadTargetBound = true;
   },
 
+  @bind
   _handleUploadError(file, error, response) {
     this._inProgressUploads--;
     this._resetUpload(file, { removePlaceholder: true });
@@ -411,15 +419,8 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
     }
   },
 
-  _bindFileInputChangeListener() {
+  @bind
-    this.fileInputEventListener = bindFileInputChangeListener(
+  pasteEventListener(event) {
-      this.fileInputEl,
-      this._addFiles.bind(this)
-    );
-  },
-
-  _bindPasteListener() {
-    this.pasteEventListener = function pasteListener(event) {
     if (
       document.activeElement !== document.querySelector(this.editorInputClass)
     ) {
@@ -438,11 +439,9 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
     if (event && event.clipboardData && event.clipboardData.files) {
       this._addFiles([...event.clipboardData.files], { pasted: true });
     }
-    }.bind(this);
-
-    this.element.addEventListener("paste", this.pasteEventListener);
   },
 
+  @bind
   _addFiles(files, opts = {}) {
     files = Array.isArray(files) ? files : [files];
     try {

app/assets/javascripts/discourse/app/mixins/textarea-text-manipulation.js

@@ -6,6 +6,7 @@ import {
   determinePostReplaceSelection,
   safariHacksDisabled,
 } from "discourse/lib/utilities";
+import { bind } from "discourse-common/utils/decorators";
 import { next, schedule } from "@ember/runloop";
 
 const isInside = (text, regex) => {
@@ -223,6 +224,7 @@ export default Mixin.create({
     return null;
   },
 
+  @bind
   paste(e) {
     if (!this._$textarea.is(":focus") && !isTesting()) {
       return;

app/assets/javascripts/discourse/app/models/private-message-topic-tracking-state.js

@@ -1,6 +1,6 @@
 import EmberObject from "@ember/object";
 import { ajax } from "discourse/lib/ajax";
-import { on } from "discourse-common/utils/decorators";
+import { bind, on } from "discourse-common/utils/decorators";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import { deepEqual, deepMerge } from "discourse-common/lib/object";
 import {
@@ -64,21 +64,23 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({
     let filterFn;
 
     if (inbox === "user") {
-      filterFn = this._isPersonal.bind(this);
+      filterFn = this._isPersonal;
     } else if (inbox === "group") {
-      filterFn = this._isGroup.bind(this);
+      filterFn = this._isGroup;
     }
 
     return Array.from(this.states.values()).filter((topic) => {
-      return (
+      return typeFilterFn(topic) && filterFn?.(topic, opts.groupName);
-        typeFilterFn(topic) && (!filterFn || filterFn(topic, opts.groupName))
-      );
     }).length;
   },
 
   trackIncoming(inbox, filter, group) {
-    this.setProperties({ inbox, filter, activeGroup: group });
+    this.setProperties({
-    this.set("isTrackingIncoming", true);
+      inbox,
+      filter,
+      activeGroup: group,
+      isTrackingIncoming: true,
+    });
   },
 
   resetIncomingTracking() {
@@ -130,6 +132,7 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({
     );
   },
 
+  @bind
   _isPersonal(topic) {
     const groups = this.currentUser?.groups;
 
@@ -142,6 +145,7 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({
     });
   },
 
+  @bind
   _isGroup(topic, activeGroupName) {
     return this.currentUser.groups.some((group) => {
       return (
@@ -151,6 +155,7 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({
     });
   },
 
+  @bind
   _processMessage(message) {
     switch (message.message_type) {
       case "new_topic":
@@ -212,7 +217,7 @@ const PrivateMessageTopicTrackingState = EmberObject.extend({
   },
 
   _notifyIncoming(topicId) {
-    if (this.isTrackingIncoming && this.newIncoming.indexOf(topicId) === -1) {
+    if (this.isTrackingIncoming && !this.newIncoming.includes(topicId)) {
       this.newIncoming.pushObject(topicId);
     }
   },

app/assets/javascripts/discourse/app/models/topic-tracking-state.js

@@ -1,5 +1,5 @@
 import EmberObject, { get } from "@ember/object";
-import discourseComputed, { on } from "discourse-common/utils/decorators";
+import discourseComputed, { bind, on } from "discourse-common/utils/decorators";
 import Category from "discourse/models/category";
 import { deepEqual, deepMerge } from "discourse-common/lib/object";
 import DiscourseURL from "discourse/lib/url";
@@ -65,15 +65,12 @@ const TopicTrackingState = EmberObject.extend({
    * @method establishChannels
    */
   establishChannels() {
-    this.messageBus.subscribe("/new", this._processChannelPayload.bind(this));
+    this.messageBus.subscribe("/new", this._processChannelPayload);
-    this.messageBus.subscribe(
+    this.messageBus.subscribe("/latest", this._processChannelPayload);
-      "/latest",
-      this._processChannelPayload.bind(this)
-    );
     if (this.currentUser) {
       this.messageBus.subscribe(
-        "/unread/" + this.currentUser.get("id"),
+        `/unread/${this.currentUser.id}`,
-        this._processChannelPayload.bind(this)
+        this._processChannelPayload
       );
     }
 
@@ -414,7 +411,7 @@ const TopicTrackingState = EmberObject.extend({
 
     // make sure all the state is up to date with what is accurate
     // from the server
-    list.topics.forEach(this._syncStateFromListTopic.bind(this));
+    list.topics.forEach(this._syncStateFromListTopic);
 
     // correct missing states, safeguard in case message bus is corrupt
     if (this._shouldCompensateState(list, filter, queryParams)) {
@@ -651,6 +648,7 @@ const TopicTrackingState = EmberObject.extend({
   // this updates the topic in the state to match the
   // topic from the list (e.g. updates category, highest read post
   // number, tags etc.)
+  @bind
   _syncStateFromListTopic(topic) {
     const state = this.findState(topic.id) || {};
 
@@ -739,6 +737,7 @@ const TopicTrackingState = EmberObject.extend({
   },
 
   // processes the data sent via messageBus, called by establishChannels
+  @bind
   _processChannelPayload(data) {
     if (["muted", "unmuted"].includes(data.message_type)) {
       this.trackMutedOrUnmutedTopic(data);

app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js

@@ -11,6 +11,7 @@ import {
 } from "discourse/tests/helpers/qunit-helpers";
 import { fixturesByUrl } from "discourse/tests/helpers/create-pretender";
 import selectKit from "../helpers/select-kit-helper";
+import { cloneJSON } from "discourse-common/lib/object";
 
 acceptance(
   "User Private Messages - user with no group messages",
@@ -83,7 +84,7 @@ acceptance(
       });
 
       server.get("/t/13.json", () => {
-        const response = { ...fixturesByUrl["/t/12/1.json"] };
+        const response = cloneJSON(fixturesByUrl["/t/12/1.json"]);
         response.suggested_group_name = "awesome_group";
         return helper.response(response);
       });
@@ -391,7 +392,7 @@ acceptance(
       assert.ok(exists(".show-mores"), "displays the topic incoming info");
     });
 
-    test("incoming unread messages while viewing group unread", async function (assert) {
+    test("incoming unread and new messages while viewing group unread", async function (assert) {
       await visit("/u/charlie/messages/group/awesome_group/unread");
 
       publishUnreadToMessageBus({ groupIds: [14], topicId: 1 });

app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js

@@ -485,7 +485,7 @@ export function exists(selector) {
 export function publishToMessageBus(channelPath, ...args) {
   MessageBus.callbacks
     .filterBy("channel", channelPath)
-    .map((c) => c.func(...args));
+    .forEach((c) => c.func(...args));
 }
 
 export async function selectText(selector, endOffset = null) {

app/assets/javascripts/discourse/tests/unit/lib/uppy-media-optimization-plugin-test.js

@@ -52,12 +52,15 @@ module("Unit | Utility | UppyMediaOptimization Plugin", function () {
     const plugin = new UppyMediaOptimization(fakeUppy, {
       runParallel: true,
     });
-    plugin._optimizeParallel = function () {
+
-      return "using parallel";
+    Object.defineProperty(plugin, "_optimizeParallel", {
-    };
+      value: () => "using parallel",
-    plugin._optimizeSerial = function () {
+    });
-      return "using serial";
+
-    };
+    Object.defineProperty(plugin, "_optimizeSerial", {
+      value: () => "using serial",
+    });
+
     plugin.install();
     assert.strictEqual(plugin.uppy.preprocessors[0](), "using parallel");
     plugin.runParallel = false;