DEV: Replace Ember implicit injections with base class extensions (#21417)

Ember's implicit injections feature is removed in Ember 4.x. We want to give ourselves more time to migrate to explicit injections, so this commit re-implements our implicit injections as extensions to the base framework classes.

Incremental migration to newer patterns can be achieved using the `@disableImplicitInjections` class decorator (available from `discourse/lib/implicit-injections').

This resolves and unsilences the `implicit-injections` deprecation.

app/assets/javascripts/discourse/app/lib/implicit-injections.js

@@ -0,0 +1,126 @@
+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 = `__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:message-bus",
+    siteSettings: "service:site-settings",
+    topicTrackingState: "service:topic-tracking-state",
+    keyValueStore: "service:key-value-store",
+    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;
+}

app/assets/javascripts/discourse/app/pre-initializers/inject-discourse-objects.js

@@ -6,26 +6,7 @@ import Session from "discourse/models/session";
 import Site from "discourse/models/site";
 import User from "discourse/models/user";
 
-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,
-  });
-}
+import { registerDiscourseImplicitInjections } from "discourse/lib/implicit-injections";
 
 export default {
   name: "inject-discourse-objects",
@@ -58,58 +39,7 @@ export default {
 
     app.register("location:discourse-location", DiscourseLocation);
 
-    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",
-      });
-    }
+    registerDiscourseImplicitInjections();
 
     startTracking(this.topicTrackingState);
   },

app/assets/javascripts/discourse/app/services/presence.js

@@ -1,4 +1,4 @@
-import Service from "@ember/service";
+import Service, { inject as 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,6 +14,7 @@ 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;
@@ -250,7 +251,12 @@ 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() {

app/assets/javascripts/discourse/config/deprecation-workflow.js

@@ -3,7 +3,6 @@ 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" },

app/assets/javascripts/discourse/tests/acceptance/implicit-injections-test.js

@@ -0,0 +1,56 @@
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
+import { test } from "qunit";
+import RestModel from "discourse/models/rest";
+import Service, { inject as service } from "@ember/service";
+import { getOwner, setOwner } from "@ember/application";
+
+acceptance("Implicit injections shims", function () {
+  test("it provides legacy injections on common models", function (assert) {
+    const serviceInstance = Service.create();
+    setOwner(serviceInstance, getOwner(this));
+
+    assert.strictEqual(
+      serviceInstance.session,
+      getOwner(this).lookup("service:session")
+    );
+  });
+
+  test("it allows overlaying explicit injections", function (assert) {
+    class MyService extends Service {
+      @service session;
+    }
+    const serviceInstance = MyService.create();
+    setOwner(serviceInstance, getOwner(this));
+
+    assert.strictEqual(
+      serviceInstance.session,
+      getOwner(this).lookup("service:session")
+    );
+  });
+
+  test("it allows overriding values by assignment", function (assert) {
+    const serviceInstance = Service.create({ session: "an override" });
+    setOwner(serviceInstance, getOwner(this));
+
+    assert.strictEqual(serviceInstance.session, "an override");
+  });
+
+  test("passes through assigned values when creating from another object", async function (assert) {
+    const initialModel = RestModel.create({
+      appEvents: "overridden app events",
+    });
+
+    assert.strictEqual(
+      initialModel.appEvents,
+      "overridden app events",
+      "overridden app events are set correctly"
+    );
+
+    const newModel = RestModel.create(initialModel);
+    assert.strictEqual(
+      newModel.appEvents,
+      "overridden app events",
+      "overriden app events are passed though when creating from another objects"
+    );
+  });
+});

app/assets/javascripts/discourse/tests/helpers/component-test.js

@@ -7,7 +7,6 @@ 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);
@@ -50,12 +49,6 @@ 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(
@@ -63,11 +56,6 @@ 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");

app/assets/javascripts/discourse/tests/unit/routes/review-index-test.js

@@ -11,7 +11,6 @@ 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");
@@ -42,7 +41,6 @@ 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");