debug info and listener targets

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

@@ -26,7 +26,7 @@ import {
   fetchUnseenMentions,
   linkSeenMentions,
 } from "discourse/lib/link-mentions";
-import { next, schedule, throttle } from "@ember/runloop";
+import { schedule, throttle } from "@ember/runloop";
 import discourseLater from "discourse-common/lib/later";
 import Component from "@ember/component";
 import Composer from "discourse/models/composer";
@@ -38,7 +38,6 @@ import { ajax } from "discourse/lib/ajax";
 import discourseDebounce from "discourse-common/lib/debounce";
 import { findRawTemplate } from "discourse-common/lib/raw-templates";
 import { iconHTML } from "discourse-common/lib/icon-library";
-import { isTesting } from "discourse-common/config/environment";
 import { loadOneboxes } from "discourse/lib/load-oneboxes";
 import putCursorAtEnd from "discourse/lib/put-cursor-at-end";
 import userSearch from "discourse/lib/user-search";
@@ -798,13 +797,13 @@ export default Component.extend(
     _composerClosed() {
       this._unbindMobileUploadButton();
       this.appEvents.trigger(`${this.composerEventPrefix}:will-close`);
-      next(() => {
-        // need to wait a bit for the "slide down" transition of the composer
-        discourseLater(
-          () => this.appEvents.trigger(`${this.composerEventPrefix}:closed`),
-          isTesting() ? 0 : 400
-        );
-      });
+
+      // need to wait a bit for the "slide down" transition of the composer
+      discourseLater(() => {
+        if (!this.isDestroying && !this.isDestroyed) {
+          this.appEvents.trigger(`${this.composerEventPrefix}:closed`);
+        }
+      }, 400);
 
       this.element
         .querySelector(".d-editor-input")

app/assets/javascripts/discourse/app/components/mount-widget.js

@@ -25,7 +25,6 @@ export default Component.extend({
   _widgetClass: null,
   _renderCallback: null,
   _childEvents: null,
-  _dispatched: null,
   dirtyKeys: null,
 
   init() {
@@ -49,7 +48,6 @@ export default Component.extend({
     this._childEvents = [];
     this._connected = [];
     this._childComponents = ArrayProxy.create({ content: [] });
-    this._dispatched = [];
     this.dirtyKeys = new DirtyKeys(name);
   },
 
@@ -76,10 +74,6 @@ export default Component.extend({
   },
 
   willDestroyElement() {
-    this._dispatched.forEach((evt) => {
-      const [eventName, caller] = evt;
-      this.appEvents.off(eventName, this, caller);
-    });
     cancel(this._timeout);
   },
 
@@ -99,10 +93,9 @@ export default Component.extend({
   dispatch(eventName, key) {
     this._childEvents.push(eventName);
 
-    const caller = (refreshArg) =>
-      this.eventDispatched(eventName, key, refreshArg);
-    this._dispatched.push([eventName, caller]);
-    this.appEvents.on(eventName, this, caller);
+    this.appEvents.on(eventName, this, (refreshArg) =>
+      this.eventDispatched(eventName, key, refreshArg)
+    );
   },
 
   queueRerender(callback) {

app/assets/javascripts/discourse/app/components/site-header.js

@@ -249,8 +249,8 @@ const SiteHeaderComponent = MountWidget.extend(
 
       this.appEvents.on("header:show-topic", this, "setTopic");
       this.appEvents.on("header:hide-topic", this, "setTopic");
-
       this.appEvents.on("user-menu:rendered", this, "_animateMenu");
+      this.appEvents.on("dom:clean", this, "_cleanDom");
 
       if (this._dropDownHeaderEnabled()) {
         this.appEvents.on(
@@ -264,11 +264,7 @@ const SiteHeaderComponent = MountWidget.extend(
       this.dispatch("header:keyboard-trigger", "header");
       this.dispatch("user-menu:navigation", "user-menu");
 
-      this.appEvents.on("dom:clean", this, "_cleanDom");
-
-      if (this.currentUser) {
-        this.currentUser.on("status-changed", this, "queueRerender");
-      }
+      this.currentUser?.on("status-changed", this, "queueRerender");
 
       const header = document.querySelector("header.d-header");
       this._itsatrap = new ItsATrap(header);

app/assets/javascripts/discourse/app/components/topic-navigation.js

@@ -211,10 +211,9 @@ export default Component.extend(PanEvents, {
 
     this._lastTopicId = this.topic.id;
 
-    this.appEvents
-      .on("topic:current-post-scrolled", this, this._topicScrolled)
-      .on("topic:jump-to-post", this, this._collapseFullscreen)
-      .on("topic:keyboard-trigger", this, this.keyboardTrigger);
+    this.appEvents.on("topic:current-post-scrolled", this, this._topicScrolled);
+    this.appEvents.on("topic:jump-to-post", this, this._collapseFullscreen);
+    this.appEvents.on("topic:keyboard-trigger", this, this.keyboardTrigger);
 
     if (!this.site.mobileView) {
       $(window).on("resize.discourse-topic-navigation", () =>
@@ -234,11 +233,6 @@ export default Component.extend(PanEvents, {
   willDestroyElement() {
     this._super(...arguments);
 
-    this.appEvents
-      .off("topic:current-post-scrolled", this, this._topicScrolled)
-      .off("topic:jump-to-post", this, this._collapseFullscreen)
-      .off("topic:keyboard-trigger", this, this.keyboardTrigger);
-
     $(window).off("click.hide-fullscreen");
 
     if (!this.site.mobileView) {

app/assets/javascripts/discourse/app/components/topic-progress.js

@@ -67,9 +67,8 @@ export default Component.extend({
   didInsertElement() {
     this._super(...arguments);
 
-    this.appEvents
-      .on("composer:resized", this, this._composerEvent)
-      .on("topic:current-post-scrolled", this, this._topicScrolled);
+    this.appEvents.on("composer:resized", this, this._composerEvent);
+    this.appEvents.on("topic:current-post-scrolled", this, this._topicScrolled);
 
     if (this.prevEvent) {
       scheduleOnce("afterRender", this, this._topicScrolled, this.prevEvent);
@@ -84,9 +83,6 @@ export default Component.extend({
   willDestroyElement() {
     this._super(...arguments);
     this._topicBottomObserver?.disconnect();
-    this.appEvents
-      .off("composer:resized", this, this._composerEvent)
-      .off("topic:current-post-scrolled", this, this._topicScrolled);
   },
 
   @bind

app/assets/javascripts/discourse/app/instance-initializers/badging.js

@@ -3,19 +3,21 @@ export default {
   after: "message-bus",
 
   initialize(owner) {
+    // must have the Badging API
     if (!navigator.setAppBadge) {
       return;
-    } // must have the Badging API
+    }
 
     const user = owner.lookup("service:current-user");
+    // must be logged in
     if (!user) {
       return;
-    } // must be logged in
+    }
 
     const appEvents = owner.lookup("service:app-events");
-    appEvents.on("notifications:changed", () => {
-      let notifications;
-      notifications = user.all_unread_notifications_count;
+    appEvents.on("notifications:changed", this, () => {
+      let notifications = user.all_unread_notifications_count;
+
       if (user.unseen_reviewable_count) {
         notifications += user.unseen_reviewable_count;
       }

app/assets/javascripts/discourse/app/instance-initializers/meta-tag-updater.js

@@ -13,7 +13,7 @@ export default {
     const canonicalUrl = document.querySelector("link[rel='canonical']");
 
     const appEvents = owner.lookup("service:app-events");
-    appEvents.on("page:changed", ({ title, url }) => {
+    appEvents.on("page:changed", this, ({ title, url }) => {
       const absoluteUrl = getAbsoluteURL(url);
 
       ogTitle?.setAttribute("content", title);

app/assets/javascripts/discourse/app/instance-initializers/page-tracking.js

@@ -13,15 +13,15 @@ export default {
     const router = owner.lookup("router:main");
     router.on("routeWillChange", this.handleRouteWillChange);
 
-    let appEvents = owner.lookup("service:app-events");
-    let documentTitle = owner.lookup("service:document-title");
+    const appEvents = owner.lookup("service:app-events");
+    const documentTitle = owner.lookup("service:document-title");
 
     startPageTracking(router, appEvents, documentTitle);
 
     // Out of the box, Discourse tries to track google analytics
     // if it is present
     if (typeof window._gaq !== "undefined") {
-      appEvents.on("page:changed", (data) => {
+      appEvents.on("page:changed", this, (data) => {
         if (!data.replacedOnlyQueryParams) {
           window._gaq.push(["_set", "title", data.title]);
           window._gaq.push(["_trackPageview", data.url]);
@@ -35,7 +35,7 @@ export default {
       typeof window.ga !== "undefined" &&
       typeof window.gtag === "undefined"
     ) {
-      appEvents.on("page:changed", (data) => {
+      appEvents.on("page:changed", this, (data) => {
         if (!data.replacedOnlyQueryParams) {
           window.ga("send", "pageview", { page: data.url, title: data.title });
         }
@@ -44,7 +44,7 @@ export default {
 
     // And Universal Analytics v4 if we're upgraded
     if (typeof window.gtag !== "undefined") {
-      appEvents.on("page:changed", (data) => {
+      appEvents.on("page:changed", this, (data) => {
         if (!data.replacedOnlyQueryParams) {
           window.gtag("event", "page_view", {
             page_location: data.url,
@@ -56,7 +56,7 @@ export default {
 
     // Google Tag Manager too
     if (typeof window.dataLayer !== "undefined") {
-      appEvents.on("page:changed", (data) => {
+      appEvents.on("page:changed", this, (data) => {
         if (!data.replacedOnlyQueryParams) {
           googleTagManagerPageChanged(data);
         }

app/assets/javascripts/discourse/app/lib/plugin-api.js

@@ -734,7 +734,7 @@ class PluginApi {
    **/
   onAppEvent(name, fn) {
     const appEvents = this._lookupContainer("service:app-events");
-    appEvents && appEvents.on(name, fn);
+    appEvents?.on(name, this, fn);
   }
 
   /**

app/assets/javascripts/discourse/app/lib/sticky-avatars.js

@@ -26,9 +26,9 @@ export default class StickyAvatars {
     }
 
     const appEvents = getOwner(this).lookup("service:app-events");
-    appEvents.on("topic:current-post-scrolled", this._handlePostNodes);
-    appEvents.on("topic:scrolled", this._handleScroll);
-    appEvents.on("page:topic-loaded", this._initIntersectionObserver);
+    appEvents.on("topic:current-post-scrolled", this, this._handlePostNodes);
+    appEvents.on("topic:scrolled", this, this._handleScroll);
+    appEvents.on("page:topic-loaded", this, this._initIntersectionObserver);
 
     addWidgetCleanCallback("post-stream", this._clearIntersectionObserver);
 

app/assets/javascripts/discourse/app/mixins/rerender-on-do-not-disturb-change.js

@@ -24,7 +24,10 @@ export default Mixin.create({
   didInsertElement() {
     this._super(...arguments);
 
-    this.appEvents.on("do-not-disturb:changed", () => this.queueRerender());
+    this.appEvents.on("do-not-disturb:changed", this, () =>
+      this.queueRerender()
+    );
+
     if (!isTesting()) {
       this.listenForDoNotDisturbChanges();
     }

app/assets/javascripts/discourse/app/services/app-events.js

@@ -1,8 +1,11 @@
+/* eslint-disable no-console */
 import Service from "@ember/service";
 import { registerDestructor } from "@ember/destroyable";
+import { DEBUG } from "@glimmer/env";
 
 export default class AppEvents extends Service {
   #listeners = new Map();
+  #usage = new Map();
 
   constructor() {
     super(...arguments);
@@ -13,12 +16,38 @@ export default class AppEvents extends Service {
     }
   }
 
+  willDestroy() {
+    super.willDestroy(...arguments);
+
+    if (DEBUG) {
+      setTimeout(() => {
+        if (this.#usage.size > 0) {
+          console.log("Leftover app-events listeners:");
+          for (const [name, count] of this.#usage) {
+            console.log(name, count);
+          }
+
+          this.#usage.clear();
+          this.#listeners.clear();
+        }
+      }, 1);
+    } else {
+      this.#listeners.clear();
+    }
+  }
+
   on(name, target, method, { once = false } = {}) {
     const listeners = this.#listeners.get(name) || [];
 
     if (typeof target === "object") {
       registerDestructor(target, () => this.off(name, target, method));
     } else {
+      if (DEBUG) {
+        console.log(
+          `Called appEvents.on("${name}", ...) without a target argument`
+        );
+      }
+
       method = target;
       target = globalThis;
     }
@@ -26,6 +55,9 @@ export default class AppEvents extends Service {
     listeners.push({ target, method, once });
     this.#listeners.set(name, listeners);
 
+    const count = this.#usage.get(name) || 0;
+    this.#usage.set(name, count + 1);
+
     return this;
   }
 
@@ -67,9 +99,23 @@ export default class AppEvents extends Service {
     }
 
     const newListeners = listeners.filter(
-      ({ _target, _method }) => !(target === _target && method === _method)
+      (listener) => !(target === listener.target && method === listener.method)
     );
-    this.#listeners.set(name, newListeners);
+
+    if (listeners.length === newListeners.length) {
+      console.warn(
+        "Trying to remove an app-event listener that doesn't exist:",
+        name
+      );
+    }
+
+    if (newListeners.length > 0) {
+      this.#listeners.set(name, newListeners);
+      this.#usage.set(name, newListeners.length);
+    } else {
+      this.#listeners.delete(name);
+      this.#usage.delete(name);
+    }
 
     return this;
   }

app/assets/javascripts/discourse/tests/unit/services/lightbox-test.js

@@ -124,7 +124,7 @@ module("Unit | Service | Experimental Lightbox", function (hooks) {
 
     const expectedEvent = LIGHTBOX_APP_EVENT_NAMES.OPEN;
 
-    this.appEvents.on(LIGHTBOX_APP_EVENT_NAMES.OPEN, (args) => {
+    this.appEvents.on(LIGHTBOX_APP_EVENT_NAMES.OPEN, this, (args) => {
       assert.deepEqual(args, expectedObject);
       done();
     });
@@ -142,7 +142,7 @@ module("Unit | Service | Experimental Lightbox", function (hooks) {
 
     await this.lightbox.setupLightboxes({ container, selector });
 
-    this.appEvents.on(LIGHTBOX_APP_EVENT_NAMES.CLOSE, () => {
+    this.appEvents.on(LIGHTBOX_APP_EVENT_NAMES.CLOSE, this, () => {
       assert.ok(true);
       done();
     });

app/assets/javascripts/discourse/tests/unit/services/screen-track-test.js

@@ -42,7 +42,7 @@ module("Unit | Service | screen-track", function (hooks) {
 
     const done = assert.async();
 
-    appEvents.on("topic:timings-sent", () => {
+    appEvents.on("topic:timings-sent", this, () => {
       assert.ok(true);
       done();
     });