From f5d391a48a63bd7d5597d621423dd020270bec45 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 4 Oct 2019 10:06:08 -0400 Subject: [PATCH] REFACTOR: Move `app-events:main` to `service:app-events` (#8152) AppEvents was always a service object in disguise, so we should move it to the correct place in the application. Doing this allows other service objects to inject it easily without container access. In the future we should also deprecate `this.appEvents` without an explicit injection too. --- app/assets/javascripts/application.js | 2 +- .../javascripts/discourse-common/resolver.js.es6 | 9 +++++++++ .../discourse/initializers/avatar-select.js.es6 | 2 +- .../discourse/initializers/badging.js.es6 | 2 +- .../discourse/initializers/page-tracking.js.es6 | 2 +- .../initializers/subscribe-user-notifications.js.es6 | 2 +- .../initializers/title-notifications.js.es6 | 2 +- .../javascripts/discourse/lib/clean-dom.js.es6 | 2 +- .../discourse/lib/keyboard-shortcuts.js.es6 | 2 +- .../javascripts/discourse/lib/plugin-api.js.es6 | 2 +- app/assets/javascripts/discourse/lib/url.js.es6 | 7 +++++++ .../pre-initializers/inject-discourse-objects.js.es6 | 7 +------ .../discourse/{lib => services}/app-events.js.es6 | 2 +- .../javascripts/discourse/widgets/widget.js.es6 | 2 +- .../initializers/new-user-narrative.js.es6 | 2 +- test/javascripts/components/d-editor-test.js.es6 | 6 +++--- test/javascripts/controllers/topic-test.js.es6 | 12 ++++++------ test/javascripts/helpers/component-test.js.es6 | 7 +------ test/javascripts/models/composer-test.js.es6 | 2 +- test/javascripts/test_helper.js | 2 +- 20 files changed, 41 insertions(+), 35 deletions(-) rename app/assets/javascripts/discourse/{lib => services}/app-events.js.es6 (95%) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 180df64af2..033e043d43 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -19,7 +19,7 @@ //= require ./discourse/lib/hash //= require ./discourse/lib/load-script //= require ./discourse/lib/notification-levels -//= require ./discourse/lib/app-events +//= require ./discourse/services/app-events //= require ./discourse/lib/offset-calculator //= require ./discourse/lib/lock-on //= require ./discourse/lib/url diff --git a/app/assets/javascripts/discourse-common/resolver.js.es6 b/app/assets/javascripts/discourse-common/resolver.js.es6 index 1b9dab0025..aeab2d45fa 100644 --- a/app/assets/javascripts/discourse-common/resolver.js.es6 +++ b/app/assets/javascripts/discourse-common/resolver.js.es6 @@ -1,4 +1,5 @@ import { findHelper } from "discourse-common/lib/helpers"; +import deprecated from "discourse-common/lib/deprecated"; /* global requirejs, require */ var classify = Ember.String.classify; @@ -45,6 +46,14 @@ export function buildResolver(baseName) { }, normalize(fullName) { + if (fullName === "app-events:main") { + deprecated( + "`app-events:main` has been replaced with `service:app-events`", + { since: "2.4.0" } + ); + return "service:app-events"; + } + const split = fullName.split(":"); if (split.length > 1) { const appBase = `${baseName}/${split[0]}s/`; diff --git a/app/assets/javascripts/discourse/initializers/avatar-select.js.es6 b/app/assets/javascripts/discourse/initializers/avatar-select.js.es6 index f35ee67540..9eaac83db9 100644 --- a/app/assets/javascripts/discourse/initializers/avatar-select.js.es6 +++ b/app/assets/javascripts/discourse/initializers/avatar-select.js.es6 @@ -10,7 +10,7 @@ export default { ).selectable_avatars_enabled; container - .lookup("app-events:main") + .lookup("service:app-events") .on("show-avatar-select", this, "_showAvatarSelect"); }, diff --git a/app/assets/javascripts/discourse/initializers/badging.js.es6 b/app/assets/javascripts/discourse/initializers/badging.js.es6 index b80416c231..004764056e 100644 --- a/app/assets/javascripts/discourse/initializers/badging.js.es6 +++ b/app/assets/javascripts/discourse/initializers/badging.js.es6 @@ -13,7 +13,7 @@ export default { user.unread_notifications + user.unread_private_messages; container - .lookup("app-events:main") + .lookup("service:app-events") .on("notifications:changed", this, "_updateBadge"); }, diff --git a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 index f11760ec6c..71fa6f25ae 100644 --- a/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 +++ b/app/assets/javascripts/discourse/initializers/page-tracking.js.es6 @@ -17,7 +17,7 @@ export default { router.on("routeWillChange", viewTrackingRequired); router.on("routeDidChange", cleanDOM); - let appEvents = container.lookup("app-events:main"); + let appEvents = container.lookup("service:app-events"); startPageTracking(router, appEvents); diff --git a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 index b71f37aaca..d3c251bb88 100644 --- a/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 +++ b/app/assets/javascripts/discourse/initializers/subscribe-user-notifications.js.es6 @@ -18,7 +18,7 @@ export default { initialize(container) { const user = container.lookup("current-user:main"); const bus = container.lookup("message-bus:main"); - const appEvents = container.lookup("app-events:main"); + const appEvents = container.lookup("service:app-events"); if (user) { bus.subscribe("/reviewable_counts", data => { diff --git a/app/assets/javascripts/discourse/initializers/title-notifications.js.es6 b/app/assets/javascripts/discourse/initializers/title-notifications.js.es6 index ca83d7a639..9801e1f688 100644 --- a/app/assets/javascripts/discourse/initializers/title-notifications.js.es6 +++ b/app/assets/javascripts/discourse/initializers/title-notifications.js.es6 @@ -9,7 +9,7 @@ export default { this.container = container; container - .lookup("app-events:main") + .lookup("service:app-events") .on("notifications:changed", this, "_updateTitle"); }, diff --git a/app/assets/javascripts/discourse/lib/clean-dom.js.es6 b/app/assets/javascripts/discourse/lib/clean-dom.js.es6 index 59157189f6..8d83f4d7f8 100644 --- a/app/assets/javascripts/discourse/lib/clean-dom.js.es6 +++ b/app/assets/javascripts/discourse/lib/clean-dom.js.es6 @@ -30,7 +30,7 @@ function _clean() { } // TODO: Avoid container lookup here - const appEvents = Discourse.__container__.lookup("app-events:main"); + const appEvents = Discourse.__container__.lookup("service:app-events"); appEvents.trigger("dom:clean"); } diff --git a/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 b/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 index 664a1cbdc6..e051f479ff 100644 --- a/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 +++ b/app/assets/javascripts/discourse/lib/keyboard-shortcuts.js.es6 @@ -86,7 +86,7 @@ export default { this._stopCallback(); this.searchService = this.container.lookup("search-service:main"); - this.appEvents = this.container.lookup("app-events:main"); + this.appEvents = this.container.lookup("service:app-events"); this.currentUser = this.container.lookup("current-user:main"); let siteSettings = this.container.lookup("site-settings:main"); diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 index 950fdcd166..4cf670b3de 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 +++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6 @@ -449,7 +449,7 @@ class PluginApi { ``` **/ onAppEvent(name, fn) { - const appEvents = this._lookupContainer("app-events:main"); + const appEvents = this._lookupContainer("service:app-events"); appEvents && appEvents.on(name, fn); } diff --git a/app/assets/javascripts/discourse/lib/url.js.es6 b/app/assets/javascripts/discourse/lib/url.js.es6 index 66b82829ec..e25f0a77fe 100644 --- a/app/assets/javascripts/discourse/lib/url.js.es6 +++ b/app/assets/javascripts/discourse/lib/url.js.es6 @@ -398,6 +398,9 @@ const DiscourseURL = Ember.Object.extend({ ); }, + // TODO: These container calls can be replaced eventually if we migrate this to a service + // object. + /** @private @@ -410,6 +413,10 @@ const DiscourseURL = Ember.Object.extend({ return Discourse.__container__.lookup("router:main"); }, + get appEvents() { + return Discourse.__container__.lookup("service:app-events"); + }, + // Get a controller. Note that currently it uses `__container__` which is not // advised but there is no other way to access the router. controllerFor(name) { diff --git a/app/assets/javascripts/discourse/pre-initializers/inject-discourse-objects.js.es6 b/app/assets/javascripts/discourse/pre-initializers/inject-discourse-objects.js.es6 index 7e1d8142e7..2555cf6d12 100644 --- a/app/assets/javascripts/discourse/pre-initializers/inject-discourse-objects.js.es6 +++ b/app/assets/javascripts/discourse/pre-initializers/inject-discourse-objects.js.es6 @@ -1,8 +1,6 @@ import Session from "discourse/models/session"; import KeyValueStore from "discourse/lib/key-value-store"; -import AppEvents from "discourse/lib/app-events"; import Store from "discourse/models/store"; -import DiscourseURL from "discourse/lib/url"; import DiscourseLocation from "discourse/lib/discourse-location"; import SearchService from "discourse/services/search"; import { @@ -17,10 +15,7 @@ export default { name: "inject-discourse-objects", initialize(container, app) { - const appEvents = AppEvents.create(); - app.register("app-events:main", appEvents, { instantiate: false }); - ALL_TARGETS.forEach(t => app.inject(t, "appEvents", "app-events:main")); - DiscourseURL.appEvents = appEvents; + ALL_TARGETS.forEach(t => app.inject(t, "appEvents", "service:app-events")); // backwards compatibility: remove when plugins have updated app.register("store:main", Store); diff --git a/app/assets/javascripts/discourse/lib/app-events.js.es6 b/app/assets/javascripts/discourse/services/app-events.js.es6 similarity index 95% rename from app/assets/javascripts/discourse/lib/app-events.js.es6 rename to app/assets/javascripts/discourse/services/app-events.js.es6 index aff71a2a40..5967568ef3 100644 --- a/app/assets/javascripts/discourse/lib/app-events.js.es6 +++ b/app/assets/javascripts/discourse/services/app-events.js.es6 @@ -1,6 +1,6 @@ import deprecated from "discourse-common/lib/deprecated"; -export default Ember.Object.extend(Ember.Evented, { +export default Ember.Service.extend(Ember.Evented, { _events: {}, on() { diff --git a/app/assets/javascripts/discourse/widgets/widget.js.es6 b/app/assets/javascripts/discourse/widgets/widget.js.es6 index 6888d66209..f6f5f78b91 100644 --- a/app/assets/javascripts/discourse/widgets/widget.js.es6 +++ b/app/assets/javascripts/discourse/widgets/widget.js.es6 @@ -111,7 +111,7 @@ export default class Widget { this.currentUser = register.lookup("current-user:main"); this.capabilities = register.lookup("capabilities:main"); this.store = register.lookup("service:store"); - this.appEvents = register.lookup("app-events:main"); + this.appEvents = register.lookup("service:app-events"); this.keyValueStore = register.lookup("key-value-store:main"); // Helps debug widgets diff --git a/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 b/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 index f9b73b1335..45b68ca24f 100644 --- a/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 +++ b/plugins/discourse-narrative-bot/assets/javascripts/initializers/new-user-narrative.js.es6 @@ -3,7 +3,7 @@ import { withPluginApi } from "discourse/lib/plugin-api"; function initialize(api) { const messageBus = api.container.lookup("message-bus:main"); const currentUser = api.getCurrentUser(); - const appEvents = api.container.lookup("app-events:main"); + const appEvents = api.container.lookup("service:app-events"); api.modifyClass("component:site-header", { didInsertElement() { diff --git a/test/javascripts/components/d-editor-test.js.es6 b/test/javascripts/components/d-editor-test.js.es6 index 6419c6be41..7566ee8b16 100644 --- a/test/javascripts/components/d-editor-test.js.es6 +++ b/test/javascripts/components/d-editor-test.js.es6 @@ -704,7 +704,7 @@ testCase("replace-text event by default", async function(assert) { this.set("value", "red green blue"); await this.container - .lookup("app-events:main") + .lookup("service:app-events") .trigger("composer:replace-text", "green", "yellow"); assert.equal(this.value, "red green blue"); @@ -714,7 +714,7 @@ composerTestCase("replace-text event for composer", async function(assert) { this.set("value", "red green blue"); await this.container - .lookup("app-events:main") + .lookup("service:app-events") .trigger("composer:replace-text", "green", "yellow"); assert.equal(this.value, "red yellow blue"); @@ -800,7 +800,7 @@ composerTestCase("replace-text event for composer", async function(assert) { setTextareaSelection(textarea, start, start + len); this.container - .lookup("app-events:main") + .lookup("service:app-events") .trigger("composer:replace-text", "green", "yellow", { forceFocus: true }); Ember.run.next(() => { diff --git a/test/javascripts/controllers/topic-test.js.es6 b/test/javascripts/controllers/topic-test.js.es6 index c142ef35a7..262b300859 100644 --- a/test/javascripts/controllers/topic-test.js.es6 +++ b/test/javascripts/controllers/topic-test.js.es6 @@ -1,15 +1,15 @@ -import AppEvents from "discourse/lib/app-events"; import Topic from "discourse/models/topic"; import PostStream from "discourse/models/post-stream"; import { Placeholder } from "discourse/lib/posts-with-placeholders"; moduleFor("controller:topic", "controller:topic", { - needs: ["controller:composer", "controller:application"], + needs: [ + "controller:composer", + "controller:application", + "service:app-events" + ], beforeEach() { - this.registry.register("app-events:main", AppEvents.create(), { - instantiate: false - }); - this.registry.injection("controller", "appEvents", "app-events:main"); + this.registry.injection("controller", "appEvents", "service:app-events"); } }); diff --git a/test/javascripts/helpers/component-test.js.es6 b/test/javascripts/helpers/component-test.js.es6 index c65394643c..8ddd75012a 100644 --- a/test/javascripts/helpers/component-test.js.es6 +++ b/test/javascripts/helpers/component-test.js.es6 @@ -1,4 +1,3 @@ -import AppEvents from "discourse/lib/app-events"; import createStore from "helpers/create-store"; import { autoLoadModules } from "discourse/initializers/auto-load-modules"; import TopicTrackingState from "discourse/models/topic-tracking-state"; @@ -11,19 +10,15 @@ export default function(name, opts) { } test(name, function(assert) { - const appEvents = AppEvents.create(); this.site = Discourse.Site.current(); this.registry.register("site-settings:main", Discourse.SiteSettings, { instantiate: false }); - this.registry.register("app-events:main", appEvents, { - instantiate: false - }); this.registry.register("capabilities:main", Ember.Object); this.registry.register("site:main", this.site, { instantiate: false }); this.registry.injection("component", "siteSettings", "site-settings:main"); - this.registry.injection("component", "appEvents", "app-events:main"); + this.registry.injection("component", "appEvents", "service:app-events"); this.registry.injection("component", "capabilities", "capabilities:main"); this.registry.injection("component", "site", "site:main"); diff --git a/test/javascripts/models/composer-test.js.es6 b/test/javascripts/models/composer-test.js.es6 index a06a99ade7..34fb578044 100644 --- a/test/javascripts/models/composer-test.js.es6 +++ b/test/javascripts/models/composer-test.js.es6 @@ -1,5 +1,5 @@ import { currentUser } from "helpers/qunit-helpers"; -import AppEvents from "discourse/lib/app-events"; +import AppEvents from "discourse/services/app-events"; import Composer from "discourse/models/composer"; import createStore from "helpers/create-store"; diff --git a/test/javascripts/test_helper.js b/test/javascripts/test_helper.js index 2d3b233478..5795795dc0 100644 --- a/test/javascripts/test_helper.js +++ b/test/javascripts/test_helper.js @@ -159,7 +159,7 @@ QUnit.testDone(function() { // ensures any event not removed is not leaking between tests // most likely in intialisers, other places (controller, component...) // should be fixed in code - var appEvents = window.Discourse.__container__.lookup("app-events:main"); + var appEvents = window.Discourse.__container__.lookup("service:app-events"); var events = appEvents.__proto__._events; Object.keys(events).forEach(function(eventKey) { var event = events[eventKey];