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

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.
This commit is contained in:
David Taylor 2023-05-03 10:39:30 +01:00 committed by GitHub
parent c1c50cb90b
commit 6ce1fbc479
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 135 additions and 88 deletions

View File

@ -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 = 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;
}

View File

@ -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);
},

View File

@ -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() {

View File

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

View File

@ -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");

View File

@ -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");