FIX: DEV: Introduce @bind decorator (#10439)

Fixes a bug in `controllers/insert-hyperlink` where `addEventListener` was called with different (anonymous) functions than the matching `removeEventListener` calls.

app/assets/javascripts/discourse-common/addon/utils/decorators.js

@@ -4,6 +4,7 @@ import extractValue from "discourse-common/utils/extract-value";
 import decoratorAlias from "discourse-common/utils/decorator-alias";
 import macroAlias from "discourse-common/utils/macro-alias";
 import { schedule, next } from "@ember/runloop";
+import { bind as emberBind } from "@ember/runloop";
 
 export default function discourseComputedDecorator(...params) {
   // determine if user called as @discourseComputed('blah', 'blah') or @discourseComputed
@@ -29,6 +30,22 @@ export function afterRender(target, name, descriptor) {
   };
 }
 
+export function bind(target, name, descriptor) {
+  return {
+    configurable: true,
+    get() {
+      const bound = emberBind(this, descriptor.value);
+      const attributes = Object.assign({}, descriptor, {
+        value: bound
+      });
+
+      Object.defineProperty(this, name, attributes);
+
+      return bound;
+    }
+  };
+}
+
 export function readOnly(target, name, desc) {
   return {
     writable: false,

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

@@ -1,9 +1,8 @@
 import Component from "@ember/component";
-import { bind } from "@ember/runloop";
 import { inject as service } from "@ember/service";
+import { bind } from "discourse-common/utils/decorators";
 
 export default Component.extend({
-  _boundFocusChange: null,
   tagName: "",
   documentTitle: service(),
 
@@ -11,10 +10,9 @@ export default Component.extend({
     this._super(...arguments);
 
     this.documentTitle.setTitle(document.title);
-    this._boundFocusChange = bind(this, this._focusChanged);
-    document.addEventListener("visibilitychange", this._boundFocusChange);
-    document.addEventListener("resume", this._boundFocusChange);
-    document.addEventListener("freeze", this._boundFocusChange);
+    document.addEventListener("visibilitychange", this._focusChanged);
+    document.addEventListener("resume", this._focusChanged);
+    document.addEventListener("freeze", this._focusChanged);
     this.session.hasFocus = true;
 
     this.appEvents.on("notifications:changed", this, this._updateNotifications);
@@ -23,10 +21,9 @@ export default Component.extend({
   willDestroyElement() {
     this._super(...arguments);
 
-    document.removeEventListener("visibilitychange", this._boundFocusChange);
-    document.removeEventListener("resume", this._boundFocusChange);
-    document.removeEventListener("freeze", this._boundFocusChange);
-    this._boundFocusChange = null;
+    document.removeEventListener("visibilitychange", this._focusChanged);
+    document.removeEventListener("resume", this._focusChanged);
+    document.removeEventListener("freeze", this._focusChanged);
 
     this.appEvents.off(
       "notifications:changed",
@@ -46,6 +43,7 @@ export default Component.extend({
     );
   },
 
+  @bind
   _focusChanged() {
     if (document.visibilityState === "hidden") {
       if (this.session.hasFocus) {

app/assets/javascripts/discourse/app/components/global-notice.js

@@ -1,6 +1,6 @@
 import getURL from "discourse-common/lib/get-url";
+import { bind } from "discourse-common/utils/decorators";
 import I18n from "I18n";
-import { bind, cancel } from "@ember/runloop";
 import Component from "@ember/component";
 import LogsNotice from "discourse/services/logs-notice";
 import EmberObject, { computed } from "@ember/object";
@@ -197,22 +197,16 @@ export default Component.extend({
   },
 
   _setupObservers() {
-    this._boundLogsNoticeHandler = bind(this, this._handleLogsNoticeUpdate);
-    LogsNotice.current().addObserver("hidden", this._boundLogsNoticeHandler);
-    LogsNotice.current().addObserver("text", this._boundLogsNoticeHandler);
+    LogsNotice.current().addObserver("hidden", this._handleLogsNoticeUpdate);
+    LogsNotice.current().addObserver("text", this._handleLogsNoticeUpdate);
   },
 
   _tearDownObservers() {
-    if (this._boundLogsNoticeHandler) {
-      LogsNotice.current().removeObserver("text", this._boundLogsNoticeHandler);
-      LogsNotice.current().removeObserver(
-        "hidden",
-        this._boundLogsNoticeHandler
-      );
-      cancel(this._boundLogsNoticeHandler);
-    }
+    LogsNotice.current().removeObserver("text", this._handleLogsNoticeUpdate);
+    LogsNotice.current().removeObserver("hidden", this._handleLogsNoticeUpdate);
   },
 
+  @bind
   _handleLogsNoticeUpdate() {
     const logNotice = Notice.create({
       text: LogsNotice.currentProp("message"),

app/assets/javascripts/discourse/app/components/pwa-install-banner.js

@@ -1,13 +1,13 @@
-import { bind } from "@ember/runloop";
 import Component from "@ember/component";
-import discourseComputed, { on } from "discourse-common/utils/decorators";
+import discourseComputed, { bind, on } from "discourse-common/utils/decorators";
 
 const USER_DISMISSED_PROMPT_KEY = "dismissed-pwa-install-banner";
 
 export default Component.extend({
   deferredInstallPromptEvent: null,
 
-  _handleInstallPromptEvent(event) {
+  @bind
+  _onInstallPrompt(event) {
     // Prevent Chrome 76+ from automatically showing the prompt
     event.preventDefault();
     // Stash the event so it can be triggered later
@@ -16,13 +16,12 @@ export default Component.extend({
 
   @on("didInsertElement")
   _registerListener() {
-    this._promptEventHandler = bind(this, this._handleInstallPromptEvent);
-    window.addEventListener("beforeinstallprompt", this._promptEventHandler);
+    window.addEventListener("beforeinstallprompt", this._onInstallPrompt);
   },
 
   @on("willDestroyElement")
   _unregisterListener() {
-    window.removeEventListener("beforeinstallprompt", this._promptEventHandler);
+    window.removeEventListener("beforeinstallprompt", this._onInstallPrompt);
   },
 
   @discourseComputed

app/assets/javascripts/discourse/app/components/share-popup.js

@@ -1,10 +1,10 @@
 import I18n from "I18n";
 import { isEmpty } from "@ember/utils";
-import { bind, scheduleOnce, later } from "@ember/runloop";
+import { scheduleOnce, later } from "@ember/runloop";
 import Component from "@ember/component";
 import { wantsNewWindow } from "discourse/lib/intercept-click";
 import { longDateNoYear } from "discourse/lib/formatter";
-import discourseComputed, { on } from "discourse-common/utils/decorators";
+import discourseComputed, { bind } from "discourse-common/utils/decorators";
 import Sharing from "discourse/lib/sharing";
 import { nativeShare } from "discourse/lib/pwa-utils";
 import { alias } from "@ember/object/computed";
@@ -94,6 +94,7 @@ export default Component.extend({
     scheduleOnce("afterRender", this, this._focusUrl);
   },
 
+  @bind
   _mouseDownHandler(event) {
     if (!this.element || this.isDestroying || this.isDestroyed) {
       return;
@@ -110,6 +111,7 @@ export default Component.extend({
     return true;
   },
 
+  @bind
   _clickHandler(event) {
     if (!this.element || this.isDestroying || this.isDestroyed) {
       return;
@@ -140,6 +142,7 @@ export default Component.extend({
     return false;
   },
 
+  @bind
   _keydownHandler(event) {
     if (!this.element || this.isDestroying || this.isDestroyed) {
       return;
@@ -154,24 +157,17 @@ export default Component.extend({
     this._showUrl($target, url);
   },
 
-  @on("init")
-  _setupHandlers() {
-    this._boundMouseDownHandler = bind(this, this._mouseDownHandler);
-    this._boundClickHandler = bind(this, this._clickHandler);
-    this._boundKeydownHandler = bind(this, this._keydownHandler);
-  },
-
   didInsertElement() {
     this._super(...arguments);
 
     $("html")
-      .on("mousedown.outside-share-link", this._boundMouseDownHandler)
+      .on("mousedown.outside-share-link", this._mouseDownHandler)
       .on(
         "click.discourse-share-link",
         "button[data-share-url], .post-info .post-date[data-share-url]",
-        this._boundClickHandler
+        this._clickHandler
       )
-      .on("keydown.share-view", this._boundKeydownHandler);
+      .on("keydown.share-view", this._keydownHandler);
 
     this.appEvents.on("share:url", this, "_shareUrlHandler");
   },
@@ -180,9 +176,9 @@ export default Component.extend({
     this._super(...arguments);
 
     $("html")
-      .off("click.discourse-share-link", this._boundClickHandler)
-      .off("mousedown.outside-share-link", this._boundMouseDownHandler)
-      .off("keydown.share-view", this._boundKeydownHandler);
+      .off("click.discourse-share-link", this._clickHandler)
+      .off("mousedown.outside-share-link", this._mouseDownHandler)
+      .off("keydown.share-view", this._keydownHandler);
 
     this.appEvents.off("share:url", this, "_shareUrlHandler");
   },

app/assets/javascripts/discourse/app/components/user-selector.js

@@ -1,5 +1,5 @@
 import { isEmpty } from "@ember/utils";
-import { on, observes } from "discourse-common/utils/decorators";
+import { bind, on, observes } from "discourse-common/utils/decorators";
 import TextField from "discourse/components/text-field";
 import userSearch from "discourse/lib/user-search";
 import { findRawTemplate } from "discourse-common/lib/raw-templates";
@@ -12,24 +12,22 @@ export default TextField.extend({
   single: false,
   fullWidthWrap: false,
 
-  init() {
-    this._super(...arguments);
+  @bind
+  _paste(event) {
+    let pastedText = "";
 
-    this._paste = e => {
-      let pastedText = "";
-      if (window.clipboardData && window.clipboardData.getData) {
-        // IE
-        pastedText = window.clipboardData.getData("Text");
-      } else if (e.clipboardData && e.clipboardData.getData) {
-        pastedText = e.clipboardData.getData("text/plain");
-      }
+    if (window.clipboardData && window.clipboardData.getData) {
+      // IE
+      pastedText = window.clipboardData.getData("Text");
+    } else if (event.clipboardData && event.clipboardData.getData) {
+      pastedText = event.clipboardData.getData("text/plain");
+    }
 
-      if (pastedText.length > 0) {
-        this.importText(pastedText);
-        e.preventDefault();
-        return false;
-      }
-    };
+    if (pastedText.length > 0) {
+      this.importText(pastedText);
+      event.preventDefault();
+      return false;
+    }
   },
 
   didUpdateAttrs() {

app/assets/javascripts/discourse/app/controllers/insert-hyperlink.js

@@ -3,6 +3,7 @@ import { cancel, debounce, schedule } from "@ember/runloop";
 import Controller from "@ember/controller";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
 import { searchForTerm } from "discourse/lib/search";
+import { bind } from "discourse-common/utils/decorators";
 
 export default Controller.extend(ModalFunctionality, {
   _debounced: null,
@@ -20,23 +21,24 @@ export default Controller.extend(ModalFunctionality, {
     schedule("afterRender", () => {
       const element = document.querySelector(".insert-link");
 
-      element.addEventListener("keydown", e => this.keyDown(e));
+      element.addEventListener("keydown", this.keyDown);
 
       element
         .closest(".modal-inner-container")
-        .addEventListener("mousedown", e => this.mouseDown(e));
+        .addEventListener("mousedown", this.mouseDown);
 
       document.querySelector("input.link-url").focus();
     });
   },
 
-  keyDown(e) {
-    switch (e.which) {
+  @bind
+  keyDown(event) {
+    switch (event.which) {
       case 40:
-        this.highlightRow(e, "down");
+        this.highlightRow(event, "down");
         break;
       case 38:
-        this.highlightRow(e, "up");
+        this.highlightRow(event, "up");
         break;
       case 13:
         // override Enter behaviour when a row is selected
@@ -45,23 +47,24 @@ export default Controller.extend(ModalFunctionality, {
             ".internal-link-results .search-link"
           )[this.selectedRow];
           this.selectLink(selected);
-          e.preventDefault();
-          e.stopPropagation();
+          event.preventDefault();
+          event.stopPropagation();
         }
         break;
       case 27:
         // Esc should cancel dropdown first
         if (this.searchResults.length) {
           this.set("searchResults", []);
-          e.preventDefault();
-          e.stopPropagation();
+          event.preventDefault();
+          event.stopPropagation();
         }
         break;
     }
   },
 
-  mouseDown(e) {
-    if (!e.target.closest(".inputs")) {
+  @bind
+  mouseDown(event) {
+    if (!event.target.closest(".inputs")) {
       this.set("searchResults", []);
     }
   },

app/assets/javascripts/discourse/app/lib/lock-on.js

@@ -1,4 +1,5 @@
 import { minimumOffset } from "discourse/lib/offset-calculator";
+import { bind } from "discourse-common/utils/decorators";
 
 // Dear traveller, you are entering a zone where we are at war with the browser.
 // The browser is insisting on positioning scrollTop per the location it was in
@@ -38,7 +39,6 @@ export default class LockOn {
   constructor(selector, options) {
     this.selector = selector;
     this.options = options || {};
-    this._boundScrollListener = this._scrollListener.bind(this);
   }
 
   elementTop() {
@@ -76,6 +76,7 @@ export default class LockOn {
     this._addListener();
   }
 
+  @bind
   _scrollListener(event) {
     if (event.which > 0 || SCROLL_TYPES.includes(event.type)) {
       this.clearLock();
@@ -87,8 +88,8 @@ export default class LockOn {
     const html = document.querySelector("html");
 
     SCROLL_EVENTS.forEach(event => {
-      body.addEventListener(event, this._boundScrollListener);
-      html.addEventListener(event, this._boundScrollListener);
+      body.addEventListener(event, this._scrollListener);
+      html.addEventListener(event, this._scrollListener);
     });
   }
 
@@ -97,8 +98,8 @@ export default class LockOn {
     const html = document.querySelector("html");
 
     SCROLL_EVENTS.forEach(event => {
-      body.removeEventListener(event, this._boundScrollListener);
-      html.removeEventListener(event, this._boundScrollListener);
+      body.removeEventListener(event, this._scrollListener);
+      html.removeEventListener(event, this._scrollListener);
     });
   }
 

app/assets/javascripts/discourse/app/lib/screen-track.js

@@ -1,6 +1,6 @@
-import { bind } from "@ember/runloop";
 import { ajax } from "discourse/lib/ajax";
 import { isTesting } from "discourse-common/config/environment";
+import { bind } from "discourse-common/utils/decorators";
 
 // We use this class to track how long posts in a topic are on the screen.
 const PAUSE_UNLESS_SCROLLED = 1000 * 60 * 3;
@@ -28,8 +28,7 @@ export default class {
     // Create an interval timer if we don't have one.
     if (!this._interval) {
       this._interval = setInterval(() => this.tick(), 1000);
-      this._boundScrolled = bind(this, this.scrolled);
-      $(window).on("scroll.screentrack", this._boundScrolled);
+      $(window).on("scroll.screentrack", this.scrolled);
     }
 
     this._topicId = topicId;
@@ -42,9 +41,7 @@ export default class {
       return;
     }
 
-    if (this._boundScrolled) {
-      $(window).off("scroll.screentrack", this._boundScrolled);
-    }
+    $(window).off("scroll.screentrack", this.scrolled);
 
     this.tick();
     this.flush();
@@ -79,6 +76,7 @@ export default class {
     this._inProgress = false;
   }
 
+  @bind
   scrolled() {
     this._lastScrolled = Date.now();
   }