From fc3a6e57e33089d56d7618a520be6a8b15f56b2f Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Mon, 15 Nov 2021 12:31:05 +0100 Subject: [PATCH] DEV: Drop support for removing all appEvent listeners (#14936) Also removes "appEventsCache". (and reduces the reported test memory usage by ~33%) There's no longer any need to remove appEvent listeners in application-instance initializers' `teardown`, as app instances are recreated before each test (in both legacy and ember cli envs) --- .../edit-notification-clicks-tracker.js | 8 +-- .../app/initializers/opengraph-tag-updater.js | 22 +++---- .../inject-discourse-objects.js | 4 +- .../discourse/app/services/app-events.js | 64 ------------------- .../discourse/tests/helpers/qunit-helpers.js | 13 ++-- .../discourse/tests/setup-tests.js | 12 +--- 6 files changed, 20 insertions(+), 103 deletions(-) diff --git a/app/assets/javascripts/discourse/app/initializers/edit-notification-clicks-tracker.js b/app/assets/javascripts/discourse/app/initializers/edit-notification-clicks-tracker.js index 8146c40218b..c410a5e0c36 100644 --- a/app/assets/javascripts/discourse/app/initializers/edit-notification-clicks-tracker.js +++ b/app/assets/javascripts/discourse/app/initializers/edit-notification-clicks-tracker.js @@ -4,15 +4,11 @@ export default { name: "edit-notification-clicks-tracker", initialize(container) { - this.appEvents = container.lookup("service:app-events"); - this.appEvents.on("edit-notification:clicked", this, this.handleClick); + const appEvents = container.lookup("service:app-events"); + appEvents.on("edit-notification:clicked", this.handleClick); }, handleClick({ topicId, postNumber, revisionNumber }) { setLastEditNotificationClick(topicId, postNumber, revisionNumber); }, - - teardown() { - this.appEvents.off("edit-notification:clicked", this, this.handleClick); - }, }; diff --git a/app/assets/javascripts/discourse/app/initializers/opengraph-tag-updater.js b/app/assets/javascripts/discourse/app/initializers/opengraph-tag-updater.js index 1d8eba4ae40..e6aa38f045a 100644 --- a/app/assets/javascripts/discourse/app/initializers/opengraph-tag-updater.js +++ b/app/assets/javascripts/discourse/app/initializers/opengraph-tag-updater.js @@ -6,21 +6,17 @@ export default { initialize(container) { // workaround for Safari on iOS 14.3 // seems it has started using opengraph tags when sharing - this.appEvents = container.lookup("service:app-events"); - this.ogTitle = document.querySelector("meta[property='og:title']"); - this.ogUrl = document.querySelector("meta[property='og:url']"); + const ogTitle = document.querySelector("meta[property='og:title']"); + const ogUrl = document.querySelector("meta[property='og:url']"); - if (this.ogTitle && this.ogUrl) { - this.appEvents.on("page:changed", this, this.updateOgAttributes); + if (!ogTitle || !ogUrl) { + return; } - }, - updateOgAttributes(data) { - this.ogTitle.setAttribute("content", data.title); - this.ogUrl.setAttribute("content", getAbsoluteURL(data.url)); - }, - - teardown() { - this.appEvents.off("page:changed", this, this.updateOgAttributes); + const appEvents = container.lookup("service:app-events"); + appEvents.on("page:changed", ({ title, url }) => { + ogTitle.setAttribute("content", title); + ogUrl.setAttribute("content", getAbsoluteURL(url)); + }); }, }; diff --git a/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js b/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js index 2149b8fb915..a34b2cfd9eb 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js @@ -14,7 +14,7 @@ import User from "discourse/models/user"; const ALL_TARGETS = ["controller", "component", "route", "model", "adapter"]; -export function registerObjects(container, app) { +export function registerObjects(app) { if (app.__registeredObjects__) { // don't run registrations twice. return; @@ -36,7 +36,7 @@ export default { after: "discourse-bootstrap", initialize(container, app) { - registerObjects(container, app); + registerObjects(app); let siteSettings = container.lookup("site-settings:main"); diff --git a/app/assets/javascripts/discourse/app/services/app-events.js b/app/assets/javascripts/discourse/app/services/app-events.js index b6ab57b068f..48008040cd7 100644 --- a/app/assets/javascripts/discourse/app/services/app-events.js +++ b/app/assets/javascripts/discourse/app/services/app-events.js @@ -1,24 +1,5 @@ import Evented from "@ember/object/evented"; import Service from "@ember/service"; -import deprecated from "discourse-common/lib/deprecated"; - -let _events = {}; - -export function clearAppEventsCache(container) { - if (container) { - const appEvents = container.lookup("service:app-events"); - Object.keys(_events).forEach((eventKey) => { - const event = _events[eventKey]; - event.forEach((listener) => { - if (appEvents.has(eventKey)) { - appEvents.off(eventKey, listener.target, listener.fn); - } - }); - }); - } - - _events = {}; -} export default Service.extend(Evented, { init() { @@ -29,49 +10,4 @@ export default Service.extend(Evented, { this.currentUser.appEvents = this; } }, - - on() { - if (arguments.length === 2) { - let [name, fn] = arguments; - let target = {}; - _events[name] = _events[name] || []; - _events[name].push({ target, fn }); - - this._super(name, target, fn); - } else if (arguments.length === 3) { - let [name, target, fn] = arguments; - _events[name] = _events[name] || []; - _events[name].push({ target, fn }); - - this._super(...arguments); - } - return this; - }, - - off() { - let name = arguments[0]; - let fn = arguments[2]; - - if (_events[name]) { - if (arguments.length === 1) { - deprecated( - "Removing all event listeners at once is deprecated, please remove each listener individually." - ); - - _events[name].forEach((ref) => { - this._super(name, ref.target, ref.fn); - }); - delete _events[name]; - } else if (arguments.length === 3) { - this._super(...arguments); - - _events[name] = _events[name].filter((e) => e.fn !== fn); - if (_events[name].length === 0) { - delete _events[name]; - } - } - } - - return this; - }, }); diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 3dd49863528..4f605fd5200 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -302,15 +302,12 @@ export function acceptance(name, optionsOrCallback) { resetLastEditNotificationClick(); clearAuthMethods(); - app._runInitializer("instanceInitializers", (initName, initializer) => { - if (initializer && initializer.teardown) { - initializer.teardown(this.container); - } + app._runInitializer("instanceInitializers", (_, initializer) => { + initializer.teardown?.(); }); - app._runInitializer("initializers", (initName, initializer) => { - if (initializer && initializer.teardown) { - initializer.teardown(this.container); - } + + app._runInitializer("initializers", (_, initializer) => { + initializer.teardown?.(this.container); }); if (LEGACY_ENV) { diff --git a/app/assets/javascripts/discourse/tests/setup-tests.js b/app/assets/javascripts/discourse/tests/setup-tests.js index 0731959f176..ec7f80b578f 100644 --- a/app/assets/javascripts/discourse/tests/setup-tests.js +++ b/app/assets/javascripts/discourse/tests/setup-tests.js @@ -12,7 +12,7 @@ import { currentSettings, resetSettings, } from "discourse/tests/helpers/site-settings"; -import { getOwner, setDefaultOwner } from "discourse-common/lib/get-owner"; +import { setDefaultOwner } from "discourse-common/lib/get-owner"; import { setApplication, setResolver } from "@ember/test-helpers"; import { setupS3CDN, setupURL } from "discourse-common/lib/get-url"; import Application from "../app"; @@ -25,7 +25,6 @@ import Session from "discourse/models/session"; import User from "discourse/models/user"; import bootbox from "bootbox"; import { buildResolver } from "discourse-common/resolver"; -import { clearAppEventsCache } from "discourse/services/app-events"; import { createHelperContext } from "discourse-common/lib/helpers"; import deprecated from "discourse-common/lib/deprecated"; import { flushMap } from "discourse/models/store"; @@ -84,7 +83,7 @@ function createApplication(config, settings) { } app.SiteSettings = settings; - registerObjects(container, app); + registerObjects(app); return app; } @@ -315,13 +314,6 @@ function setupTestsCommon(application, container, config) { $(".modal-backdrop").remove(); flushMap(); - if (isLegacyEmber()) { - // ensures any event not removed is not leaking between tests - // most likely in initializers, other places (controller, component...) - // should be fixed in code - clearAppEventsCache(getOwner(this)); - } - MessageBus.unsubscribe("*"); server = null; });