diff --git a/app/assets/javascripts/discourse/app/lib/implicit-injections.js b/app/assets/javascripts/discourse/app/lib/implicit-injections.js deleted file mode 100644 index 81e2d483d62..00000000000 --- a/app/assets/javascripts/discourse/app/lib/implicit-injections.js +++ /dev/null @@ -1,126 +0,0 @@ -import { getOwner } from "@ember/application"; -import EmberObject, { computed } from "@ember/object"; - -import Controller from "@ember/controller"; -import Component from "@ember/component"; -import Route from "@ember/routing/route"; -import RestModel from "discourse/models/rest"; -import RestAdapter from "discourse/adapters/rest"; -import Service from "@ember/service"; - -const disableImplicitInjectionsKey = Symbol("DISABLE_IMPLICIT_INJECTIONS"); - -/** - * Based on the Ember's standard injection helper, plus extra logic to make it behave more - * like Ember<=3 'implicit injections', and to allow disabling it on a per-class basis. - * https://github.com/emberjs/ember.js/blob/22b318a381/packages/%40ember/-internals/metal/lib/injected_property.ts#L37 - * - */ -function implicitInjectionShim(lookupName, key) { - let overrideKey = Symbol(`OVERRIDE_${key}`); - - return computed(key, { - get() { - if (this[overrideKey]) { - return this[overrideKey]; - } - if (this[disableImplicitInjectionsKey]) { - return undefined; - } - - let owner = getOwner(this) || this.container; - if (!owner) { - return undefined; - } - return owner.lookup(lookupName); - }, - - set(_, value) { - return (this[overrideKey] = value); - }, - }); -} - -function setInjections(target, injections) { - const extension = {}; - for (const [key, lookupName] of Object.entries(injections)) { - extension[key] = implicitInjectionShim(lookupName, key); - } - EmberObject.reopen.call(target, extension); -} - -let alreadyRegistered = false; - -/** - * Configure Discourse's standard injections on common framework classes. - * In Ember<=3 this was done using 'implicit injections', but these have been - * removed in Ember 4. This shim implements similar behaviour by reopening the - * base framework classes. - * - * Long-term we aim to move away from this pattern, towards 'explicit injections' - * https://guides.emberjs.com/release/applications/dependency-injection/ - * - * Incremental migration to newer patterns can be achieved using the `@disableImplicitInjections` - * helper (availble on `discourse/lib/implicit-injections') - */ -export function registerDiscourseImplicitInjections() { - if (alreadyRegistered) { - return; - } - const commonInjections = { - appEvents: "service:app-events", - pmTopicTrackingState: "service:pm-topic-tracking-state", - store: "service:store", - site: "service:site", - searchService: "service:search", - session: "service:session", - messageBus: "service:message-bus", - siteSettings: "service:site-settings", - topicTrackingState: "service:topic-tracking-state", - keyValueStore: "service:key-value-store", - }; - - setInjections(Controller, { - ...commonInjections, - capabilities: "service:capabilities", - currentUser: "service:current-user", - }); - - setInjections(Component, { - capabilities: "service:capabilities", - currentUser: "service:current-user", - ...commonInjections, - }); - - setInjections(Route, { - ...commonInjections, - currentUser: "service:current-user", - }); - - setInjections(RestModel, { - ...commonInjections, - }); - - setInjections(RestAdapter, { - ...commonInjections, - }); - - setInjections(Service, { - session: "service:session", - messageBus: "service:messageBus", - siteSettings: "service:site-settings", - topicTrackingState: "service:topic-tracking-state", - keyValueStore: "service:keyValueStore", - currentUser: "service:current-user", - }); - - alreadyRegistered = true; -} - -/** - * A class decorator which disables implicit injections for instances of this class. - * Essentially opts-in to the modern Ember 4+ behaviour. - */ -export function disableImplicitInjections(target) { - target.prototype[disableImplicitInjectionsKey] = true; -} 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 8fd2a0fc0b3..79744245198 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 @@ -6,7 +6,26 @@ import Session from "discourse/models/session"; import Site from "discourse/models/site"; import User from "discourse/models/user"; -import { registerDiscourseImplicitInjections } from "discourse/lib/implicit-injections"; +const ALL_TARGETS = ["controller", "component", "route", "model", "adapter"]; + +export function injectServiceIntoService({ app, property, specifier }) { + // app.inject doesn't allow implicit injection of services into services. + // However, we need to do it in order to convert our old service-like objects + // into true services, without breaking existing implicit injections. + // This hack will be removed when we remove implicit injections for the Ember 4.0 update. + + // Supplying a specific injection with the same property name prevents the infinite + // which would be caused by injecting a service into itself + app.register("discourse:null", null, { instantiate: false }); + app.inject(specifier, property, "discourse:null"); + + // Bypass the validation in `app.inject` by adding directly to the array + app.__registry__._typeInjections["service"] ??= []; + app.__registry__._typeInjections["service"].push({ + property, + specifier, + }); +} export default { name: "inject-discourse-objects", @@ -39,7 +58,58 @@ export default { app.register("location:discourse-location", DiscourseLocation); - registerDiscourseImplicitInjections(); + app.inject("controller", "capabilities", "service:capabilities"); + app.inject("component", "capabilities", "service:capabilities"); + + ALL_TARGETS.forEach((t) => { + app.inject(t, "appEvents", "service:app-events"); + app.inject(t, "pmTopicTrackingState", "service:pm-topic-tracking-state"); + app.inject(t, "store", "service:store"); + app.inject(t, "site", "service:site"); + app.inject(t, "searchService", "service:search"); + app.inject(t, "session", "service:session"); + app.inject(t, "messageBus", "service:message-bus"); + app.inject(t, "siteSettings", "service:site-settings"); + app.inject(t, "topicTrackingState", "service:topic-tracking-state"); + app.inject(t, "keyValueStore", "service:key-value-store"); + }); + + injectServiceIntoService({ + app, + property: "session", + specifier: "service:session", + }); + injectServiceIntoService({ + app, + property: "messageBus", + specifier: "service:message-bus", + }); + injectServiceIntoService({ + app, + property: "siteSettings", + specifier: "service:site-settings", + }); + injectServiceIntoService({ + app, + property: "topicTrackingState", + specifier: "service:topic-tracking-state", + }); + injectServiceIntoService({ + app, + property: "keyValueStore", + specifier: "service:key-value-store", + }); + + if (currentUser) { + ["controller", "component", "route"].forEach((t) => { + app.inject(t, "currentUser", "service:current-user"); + }); + injectServiceIntoService({ + app, + property: "currentUser", + specifier: "service:current-user", + }); + } startTracking(this.topicTrackingState); }, diff --git a/app/assets/javascripts/discourse/app/services/presence.js b/app/assets/javascripts/discourse/app/services/presence.js index 30348d22360..38abb4d4796 100644 --- a/app/assets/javascripts/discourse/app/services/presence.js +++ b/app/assets/javascripts/discourse/app/services/presence.js @@ -1,4 +1,4 @@ -import Service, { inject as service } from "@ember/service"; +import Service from "@ember/service"; import EmberObject, { computed } from "@ember/object"; import { ajax } from "discourse/lib/ajax"; import { cancel, debounce, next, once, throttle } from "@ember/runloop"; @@ -14,7 +14,6 @@ import { bind } from "discourse-common/utils/decorators"; import Evented from "@ember/object/evented"; import { isTesting } from "discourse-common/config/environment"; import getURL from "discourse-common/lib/get-url"; -import { disableImplicitInjections } from "discourse/lib/implicit-injections"; const PRESENCE_INTERVAL_S = 30; const DEFAULT_PRESENCE_DEBOUNCE_MS = isTesting() ? 0 : 500; @@ -251,12 +250,7 @@ class PresenceChannelState extends EmberObject.extend(Evented) { } } -@disableImplicitInjections export default class PresenceService extends Service { - @service currentUser; - @service siteSettings; - @service messageBus; - _presenceDebounceMs = DEFAULT_PRESENCE_DEBOUNCE_MS; init() { diff --git a/app/assets/javascripts/discourse/config/deprecation-workflow.js b/app/assets/javascripts/discourse/config/deprecation-workflow.js index 7b1f40b93a5..b7dc6257241 100644 --- a/app/assets/javascripts/discourse/config/deprecation-workflow.js +++ b/app/assets/javascripts/discourse/config/deprecation-workflow.js @@ -3,6 +3,7 @@ globalThis.deprecationWorkflow.config = { // We're using RAISE_ON_DEPRECATION in environment.js instead of // `throwOnUnhandled` here since it is easier to toggle. workflow: [ + { handler: "silence", matchId: "implicit-injections" }, { handler: "silence", matchId: "route-render-template" }, { handler: "silence", matchId: "routing.transition-methods" }, { handler: "silence", matchId: "route-disconnect-outlet" }, diff --git a/app/assets/javascripts/discourse/tests/helpers/component-test.js b/app/assets/javascripts/discourse/tests/helpers/component-test.js index a85918c7ef9..f4e36d4c988 100644 --- a/app/assets/javascripts/discourse/tests/helpers/component-test.js +++ b/app/assets/javascripts/discourse/tests/helpers/component-test.js @@ -7,6 +7,7 @@ import { autoLoadModules } from "discourse/initializers/auto-load-modules"; import QUnit, { test } from "qunit"; import { setupRenderingTest as emberSetupRenderingTest } from "ember-qunit"; import { currentSettings } from "discourse/tests/helpers/site-settings"; +import { injectServiceIntoService } from "discourse/pre-initializers/inject-discourse-objects"; export function setupRenderingTest(hooks) { emberSetupRenderingTest(hooks); @@ -49,6 +50,12 @@ export function setupRenderingTest(hooks) { this.owner.register("service:current-user", currentUser, { instantiate: false, }); + this.owner.inject("component", "currentUser", "service:current-user"); + injectServiceIntoService({ + app: this.owner.application, + property: "currentUser", + specifier: "service:current-user", + }); this.owner.unregister("service:topic-tracking-state"); this.owner.register( @@ -56,6 +63,11 @@ export function setupRenderingTest(hooks) { TopicTrackingState.create({ currentUser }), { instantiate: false } ); + 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/unit/routes/review-index-test.js b/app/assets/javascripts/discourse/tests/unit/routes/review-index-test.js index deddb95ef9d..489777c775f 100644 --- a/app/assets/javascripts/discourse/tests/unit/routes/review-index-test.js +++ b/app/assets/javascripts/discourse/tests/unit/routes/review-index-test.js @@ -11,6 +11,7 @@ module("Unit | Route | review-index", function (hooks) { this.owner.register("service:current-user", currentUser, { instantiate: false, }); + this.owner.inject("route", "currentUser", "service:current-user"); const reviewIndexRoute = this.owner.lookup("route:review-index"); const messageBus = this.owner.lookup("service:message-bus"); @@ -41,6 +42,7 @@ module("Unit | Route | review-index", function (hooks) { this.owner.register("service:current-user", currentUser, { instantiate: false, }); + this.owner.inject("route", "currentUser", "service:current-user"); const reviewIndexRoute = this.owner.lookup("route:review-index"); const messageBus = this.owner.lookup("service:message-bus");