From c1cdb6bc51873afa1c351be6bbe878616ccd341b Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 5 Aug 2022 01:48:55 +0100 Subject: [PATCH] DEV: Replace `topic-tracking-state:main` with `service:topic-tracking-state` (#17802) This will allow consumers to inject it using `topicTrackingState: service()` in preparation for the removal of implicit injections in Ember 4.0. `topic-tracking-state:main` is still available and will print a deprecation notice. Ideally we would convert topic-tracking-state into a true service, rather than registering a model instance into the registry. However, inter-dependencies between service injections make this very difficult to achieve. We don't want to block Glimmer Component work, so this commit does the minimum for now. --- .../javascripts/discourse-common/addon/resolver.js | 5 +++++ .../discourse/app/components/glimmer.js | 9 +-------- .../app/initializers/auto-load-modules.js | 2 +- .../pre-initializers/inject-discourse-objects.js | 10 +++++++--- .../javascripts/discourse/app/services/store.js | 4 +++- .../discourse/app/widgets/hamburger-menu.js | 2 +- .../acceptance/sidebar-categories-section-test.js | 4 ++-- .../acceptance/sidebar-community-section-test.js | 6 +++--- .../tests/acceptance/sidebar-tags-section-test.js | 4 ++-- .../acceptance/topic-discovery-tracked-test.js | 8 ++++---- .../discourse/tests/helpers/component-test.js | 14 +++++++------- .../discourse/tests/helpers/create-store.js | 2 +- 12 files changed, 37 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/discourse-common/addon/resolver.js b/app/assets/javascripts/discourse-common/addon/resolver.js index 5ff207f3543..88f2bdceb12 100644 --- a/app/assets/javascripts/discourse-common/addon/resolver.js +++ b/app/assets/javascripts/discourse-common/addon/resolver.js @@ -83,6 +83,11 @@ const DEPRECATED_MODULES = new Map( since: "2.9.0.beta7", dropFrom: "3.0.0", }, + "topic-tracking-state:main": { + newName: "service:topic-tracking-state", + since: "2.9.0.beta7", + dropFrom: "3.0.0", + }, }) ); diff --git a/app/assets/javascripts/discourse/app/components/glimmer.js b/app/assets/javascripts/discourse/app/components/glimmer.js index 9fce9cb0ef0..b3142a9060a 100644 --- a/app/assets/javascripts/discourse/app/components/glimmer.js +++ b/app/assets/javascripts/discourse/app/components/glimmer.js @@ -1,6 +1,4 @@ import GlimmerComponent from "@glimmer/component"; -import { cached } from "@glimmer/tracking"; -import { getOwner } from "@ember/application"; import { inject as service } from "@ember/service"; /* @@ -22,10 +20,5 @@ export default class DiscourseGlimmerComponent extends GlimmerComponent { @service currentUser; @service session; @service site; - - @cached - get topicTrackingState() { - const applicationInstance = getOwner(this); - return applicationInstance.lookup("topic-tracking-state:main"); - } + @service topicTrackingState; } diff --git a/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js b/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js index 9959c6357f9..c37a90fd5ce 100644 --- a/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js +++ b/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js @@ -24,7 +24,7 @@ export function autoLoadModules(container, registry) { currentUser: container.lookup("service:current-user"), site: container.lookup("service:site"), session: container.lookup("service:session"), - topicTrackingState: container.lookup("topic-tracking-state:main"), + topicTrackingState: container.lookup("service:topic-tracking-state"), registry, }; setOwner(context, container); 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 aa6fa57e68e..ba54502f20c 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 @@ -46,7 +46,7 @@ export default { currentUser, }); - app.register("topic-tracking-state:main", topicTrackingState, { + app.register("service:topic-tracking-state", topicTrackingState, { instantiate: false, }); @@ -67,7 +67,7 @@ export default { app.inject(t, "session", "service:session"); app.inject(t, "messageBus", "service:message-bus"); app.inject(t, "siteSettings", "service:site-settings"); - app.inject(t, "topicTrackingState", "topic-tracking-state:main"); + app.inject(t, "topicTrackingState", "service:topic-tracking-state"); app.inject(t, "keyValueStore", "service:key-value-store"); }); @@ -86,7 +86,11 @@ export default { property: "siteSettings", specifier: "service:site-settings", }); - app.inject("service", "topicTrackingState", "topic-tracking-state:main"); + injectServiceIntoService({ + app, + property: "topicTrackingState", + specifier: "service:topic-tracking-state", + }); injectServiceIntoService({ app, property: "keyValueStore", diff --git a/app/assets/javascripts/discourse/app/services/store.js b/app/assets/javascripts/discourse/app/services/store.js index bf52a63feff..57489ab0be4 100644 --- a/app/assets/javascripts/discourse/app/services/store.js +++ b/app/assets/javascripts/discourse/app/services/store.js @@ -266,7 +266,9 @@ export default Service.extend({ obj.__state = obj[adapter.primaryKey] ? "created" : "new"; // TODO: Have injections be automatic - obj.topicTrackingState = this.register.lookup("topic-tracking-state:main"); + obj.topicTrackingState = this.register.lookup( + "service:topic-tracking-state" + ); obj.keyValueStore = this.register.lookup("service:key-value-store"); const klass = this.register.lookupFactory("model:" + type) || RestModel; diff --git a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js index 463c5bf5d12..14b2848ebeb 100644 --- a/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/hamburger-menu.js @@ -95,7 +95,7 @@ export default createWidget("hamburger-menu", { }, lookupCount(type) { - const tts = this.register.lookup("topic-tracking-state:main"); + const tts = this.register.lookup("service:topic-tracking-state"); return tts ? tts.lookupCount({ type }) : 0; }, diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js index 87a9ffab83a..fb91929bf27 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js @@ -273,7 +273,7 @@ acceptance("Sidebar - Categories Section", function (needs) { test("new and unread count for categories link", async function (assert) { const { category1, category2 } = setupUserSidebarCategories(); - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -391,7 +391,7 @@ acceptance("Sidebar - Categories Section", function (needs) { await visit("/"); const topicTrackingState = this.container.lookup( - "topic-tracking-state:main" + "service:topic-tracking-state" ); const initialCallbackCount = Object.keys( diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js index 7528a9c077c..39b3c31effe 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js @@ -492,7 +492,7 @@ acceptance("Sidebar - Community Section", function (needs) { }); test("new and unread count for everything link", async function (assert) { - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -679,7 +679,7 @@ acceptance("Sidebar - Community Section", function (needs) { const category = categories.find((c) => c.id === 1001); category.set("notification_level", NotificationLevels.TRACKING); - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -923,7 +923,7 @@ acceptance("Sidebar - Community Section", function (needs) { await visit("/"); const topicTrackingState = this.container.lookup( - "topic-tracking-state:main" + "service:topic-tracking-state" ); const initialCallbackCount = Object.keys( diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js index 6ce87327b04..51737b6a28f 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js @@ -213,7 +213,7 @@ acceptance("Sidebar - Tags section", function (needs) { }); test("new and unread count for tag section links", async function (assert) { - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -319,7 +319,7 @@ acceptance("Sidebar - Tags section", function (needs) { await visit("/"); const topicTrackingState = this.container.lookup( - "topic-tracking-state:main" + "service:topic-tracking-state" ); const initialCallbackCount = Object.keys( diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js index ce0429789d3..f60f66ac0b5 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js @@ -39,7 +39,7 @@ acceptance("Topic Discovery Tracked", function (needs) { }); test("navigation items with tracked filter", async function (assert) { - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -104,7 +104,7 @@ acceptance("Topic Discovery Tracked", function (needs) { const category = categories.find((c) => c.id === 1001); category.set("notification_level", NotificationLevels.TRACKING); - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -230,7 +230,7 @@ acceptance("Topic Discovery Tracked", function (needs) { }); test("visit discovery page filtered by tag with tracked filter", async function (assert) { - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, @@ -278,7 +278,7 @@ acceptance("Topic Discovery Tracked", function (needs) { const category = categories.at(-1); category.set("notification_level", NotificationLevels.TRACKING); - this.container.lookup("topic-tracking-state:main").loadStates([ + this.container.lookup("service:topic-tracking-state").loadStates([ { topic_id: 1, highest_post_number: 1, diff --git a/app/assets/javascripts/discourse/tests/helpers/component-test.js b/app/assets/javascripts/discourse/tests/helpers/component-test.js index 3ae23e4f179..c23b4f15420 100644 --- a/app/assets/javascripts/discourse/tests/helpers/component-test.js +++ b/app/assets/javascripts/discourse/tests/helpers/component-test.js @@ -43,17 +43,17 @@ export function setupRenderingTest(hooks) { specifier: "service:current-user", }); - this.owner.unregister("topic-tracking-state:main"); + this.owner.unregister("service:topic-tracking-state"); this.owner.register( - "topic-tracking-state:main", + "service:topic-tracking-state", TopicTrackingState.create({ currentUser }), { instantiate: false } ); - this.owner.inject( - "service", - "topicTrackingState", - "topic-tracking-state:main" - ); + injectServiceIntoService({ + app: this.owner.application, + property: "topicTrackingState", + specifier: "service:topic-tracking-state", + }); autoLoadModules(this.owner, this.registry); this.owner.lookup("service:store"); diff --git a/app/assets/javascripts/discourse/tests/helpers/create-store.js b/app/assets/javascripts/discourse/tests/helpers/create-store.js index 727ee3f5f7a..61f0f74b8be 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-store.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-store.js @@ -69,7 +69,7 @@ export default function (customLookup = () => {}) { this._kvs = this._kvs || new KeyValueStore(); return this._kvs; } - if (type === "topic-tracking-state:main") { + if (type === "service:topic-tracking-state") { this._tracker = this._tracker || TopicTrackingState.create(); return this._tracker; }