From 51957c07f12627ee19d1173ae1b37489f51257f4 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 27 Jul 2022 11:38:33 +0100 Subject: [PATCH] DEV: Convert `key-value-store:main` to `service:key-value-store` (#17676) This will allow consumers to inject it using `keyValueStore: service()` in preparation for the removal of implicit injections in Ember 4.0. `key-value-store:main` is still available and will print a deprecation notice. To make this conversion possible, we have to bypass the `app.inject` logic which blocks injecting services into services. This is not ideal, but there is no other way for us to do this in a way that is backwards-compatible, and will still print a useful deprecation message when we eventually turn on the implicit-injections deprecation notice. --- .../discourse/app/components/glimmer.js | 7 +- .../discourse/app/controllers/composer.js | 4 +- .../app/initializers/auto-load-modules.js | 2 +- .../discourse/app/initializers/logs-notice.js | 2 +- .../discourse/app/initializers/signup-cta.js | 2 +- .../inject-discourse-objects.js | 97 +++++++++++++------ .../discourse/app/services/key-value-store.js | 23 +++++ .../discourse/app/services/store.js | 2 +- .../discourse/app/widgets/widget.js | 2 +- .../discourse/tests/helpers/create-store.js | 2 +- 10 files changed, 99 insertions(+), 44 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/services/key-value-store.js diff --git a/app/assets/javascripts/discourse/app/components/glimmer.js b/app/assets/javascripts/discourse/app/components/glimmer.js index cf3d4aeeb2c..62a944f215e 100644 --- a/app/assets/javascripts/discourse/app/components/glimmer.js +++ b/app/assets/javascripts/discourse/app/components/glimmer.js @@ -15,6 +15,7 @@ export default class DiscourseGlimmerComponent extends GlimmerComponent { @service appEvents; @service store; @service("search") searchService; + @service keyValueStore; @cached get siteSettings() { @@ -63,10 +64,4 @@ export default class DiscourseGlimmerComponent extends GlimmerComponent { const applicationInstance = getOwner(this); return applicationInstance.lookup("session:main"); } - - @cached - get keyValueStore() { - const applicationInstance = getOwner(this); - return applicationInstance.lookup("key-value-store:main"); - } } diff --git a/app/assets/javascripts/discourse/app/controllers/composer.js b/app/assets/javascripts/discourse/app/controllers/composer.js index e0fbdbf3964..1c183e2e2ae 100644 --- a/app/assets/javascripts/discourse/app/controllers/composer.js +++ b/app/assets/javascripts/discourse/app/controllers/composer.js @@ -185,7 +185,7 @@ export default Controller.extend({ showToolbar: computed({ get() { - const keyValueStore = getOwner(this).lookup("key-value-store:main"); + const keyValueStore = getOwner(this).lookup("service:key-value-store"); const storedVal = keyValueStore.get("toolbar-enabled"); if (this._toolbarEnabled === undefined && storedVal === undefined) { // iPhone 6 is 375, anything narrower and toolbar should @@ -197,7 +197,7 @@ export default Controller.extend({ return this._toolbarEnabled || storedVal === "true"; }, set(key, val) { - const keyValueStore = getOwner(this).lookup("key-value-store:main"); + const keyValueStore = getOwner(this).lookup("service:key-value-store"); this._toolbarEnabled = val; keyValueStore.set({ key: "toolbar-enabled", 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 3c827a9ccd7..2f2715021b5 100644 --- a/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js +++ b/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js @@ -19,7 +19,7 @@ export function autoLoadModules(container, registry) { let context = { siteSettings: container.lookup("site-settings:main"), - keyValueStore: container.lookup("key-value-store:main"), + keyValueStore: container.lookup("service:key-value-store"), capabilities: container.lookup("capabilities:main"), currentUser: container.lookup("current-user:main"), site: container.lookup("site:main"), diff --git a/app/assets/javascripts/discourse/app/initializers/logs-notice.js b/app/assets/javascripts/discourse/app/initializers/logs-notice.js index fed410eab08..9db73ba6402 100644 --- a/app/assets/javascripts/discourse/app/initializers/logs-notice.js +++ b/app/assets/javascripts/discourse/app/initializers/logs-notice.js @@ -13,7 +13,7 @@ export default { const siteSettings = container.lookup("site-settings:main"); const messageBus = container.lookup("message-bus:main"); - const keyValueStore = container.lookup("key-value-store:main"); + const keyValueStore = container.lookup("service:key-value-store"); const currentUser = container.lookup("current-user:main"); LogsNotice.reopenClass(Singleton, { createCurrent() { diff --git a/app/assets/javascripts/discourse/app/initializers/signup-cta.js b/app/assets/javascripts/discourse/app/initializers/signup-cta.js index da1854656e8..d02fbb3a5b5 100644 --- a/app/assets/javascripts/discourse/app/initializers/signup-cta.js +++ b/app/assets/javascripts/discourse/app/initializers/signup-cta.js @@ -12,7 +12,7 @@ export default { const screenTrack = container.lookup("service:screen-track"); const session = Session.current(); const siteSettings = container.lookup("site-settings:main"); - const keyValueStore = container.lookup("key-value-store:main"); + const keyValueStore = container.lookup("service:key-value-store"); const user = container.lookup("current-user:main"); const appEvents = container.lookup("service:app-events"); 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 6f82a8ca764..f5e546f3e5d 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 @@ -3,7 +3,6 @@ import TopicTrackingState, { } from "discourse/models/topic-tracking-state"; import PrivateMessageTopicTrackingState from "discourse/models/private-message-topic-tracking-state"; import DiscourseLocation from "discourse/lib/discourse-location"; -import KeyValueStore from "discourse/lib/key-value-store"; import MessageBus from "message-bus-client"; import Session from "discourse/models/session"; import Site from "discourse/models/site"; @@ -26,6 +25,37 @@ export function registerObjects(app) { app.register("site-settings:main", siteSettings, { instantiate: false }); } +function injectServiceIntoService({ container, 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. + container.lookup(specifier); + app.__registry__._typeInjections["service"].push({ + property, + specifier, + }); +} + +function deprecateRegistration({ + app, + container, + oldName, + newName, + since, + dropFrom, +}) { + app.register(oldName, { + create() { + deprecated(`"${oldName}" is deprecated, use "${newName}" instead`, { + since, + dropFrom, + }); + return container.lookup(newName); + }, + }); +} + export default { name: "inject-discourse-objects", after: "discourse-bootstrap", @@ -33,15 +63,31 @@ export default { initialize(container, app) { registerObjects(app); - app.register("store:main", { - create() { - deprecated(`"store:main" is deprecated, use "service:store" instead`, { - since: "2.8.0.beta8", - dropFrom: "2.9.0.beta1", - }); + deprecateRegistration({ + app, + container, + oldName: "store:main", + newName: "service:store", + since: "2.8.0.beta8", + dropFrom: "2.9.0.beta1", + }); - return container.lookup("service:store"); - }, + deprecateRegistration({ + app, + container, + oldName: "search-service:main", + newName: "service:search", + since: "2.8.0.beta8", + dropFrom: "2.9.0.beta1", + }); + + deprecateRegistration({ + app, + container, + oldName: "key-value-store:main", + newName: "service:key-value-store", + since: "2.9.0.beta7", + dropFrom: "3.0.0", }); let siteSettings = container.lookup("site-settings:main"); @@ -77,37 +123,28 @@ export default { app.register("location:discourse-location", DiscourseLocation); - const keyValueStore = new KeyValueStore("discourse_"); - app.register("key-value-store:main", keyValueStore, { instantiate: false }); - - app.register("search-service:main", { - create() { - deprecated( - `"search-service:main" is deprecated, use "service:search" instead`, - { - since: "2.8.0.beta8", - dropFrom: "2.9.0.beta1", - } - ); - - return container.lookup("service:search"); - }, - }); - ALL_TARGETS.forEach((t) => { app.inject(t, "appEvents", "service:app-events"); app.inject(t, "pmTopicTrackingState", "pm-topic-tracking-state:main"); app.inject(t, "store", "service:store"); app.inject(t, "site", "site:main"); app.inject(t, "searchService", "service:search"); - }); - - ALL_TARGETS.concat("service").forEach((t) => { app.inject(t, "session", "session:main"); app.inject(t, "messageBus", "message-bus:main"); app.inject(t, "siteSettings", "site-settings:main"); app.inject(t, "topicTrackingState", "topic-tracking-state:main"); - app.inject(t, "keyValueStore", "key-value-store:main"); + app.inject(t, "keyValueStore", "service:key-value-store"); + }); + + app.inject("service", "session", "session:main"); + app.inject("service", "messageBus", "message-bus:main"); + app.inject("service", "siteSettings", "site-settings:main"); + app.inject("service", "topicTrackingState", "topic-tracking-state:main"); + injectServiceIntoService({ + container, + app, + property: "keyValueStore", + specifier: "service:key-value-store", }); if (currentUser) { diff --git a/app/assets/javascripts/discourse/app/services/key-value-store.js b/app/assets/javascripts/discourse/app/services/key-value-store.js new file mode 100644 index 00000000000..91d9e0f12f6 --- /dev/null +++ b/app/assets/javascripts/discourse/app/services/key-value-store.js @@ -0,0 +1,23 @@ +import Service from "@ember/service"; +import KeyValueStore from "discourse/lib/key-value-store"; + +const PROXIED_METHODS = Object.getOwnPropertyNames( + KeyValueStore.prototype +).reject((p) => p === "constructor"); + +/** + * This is the global key-value-store which is injectable as a service. + * Alternatively, consumers can use `discourse/lib/key-value-store` directly + * to create their own namespaced store. + * */ +export default class KeyValueStoreService extends Service { + _keyValueStore = new KeyValueStore("discourse_"); + + constructor() { + super(...arguments); + + for (const name of PROXIED_METHODS) { + this[name] = this._keyValueStore[name].bind(this._keyValueStore); + } + } +} diff --git a/app/assets/javascripts/discourse/app/services/store.js b/app/assets/javascripts/discourse/app/services/store.js index 4b0cfeeb822..bf52a63feff 100644 --- a/app/assets/javascripts/discourse/app/services/store.js +++ b/app/assets/javascripts/discourse/app/services/store.js @@ -267,7 +267,7 @@ export default Service.extend({ // TODO: Have injections be automatic obj.topicTrackingState = this.register.lookup("topic-tracking-state:main"); - obj.keyValueStore = this.register.lookup("key-value-store:main"); + obj.keyValueStore = this.register.lookup("service:key-value-store"); const klass = this.register.lookupFactory("model:" + type) || RestModel; const model = klass.create(obj); diff --git a/app/assets/javascripts/discourse/app/widgets/widget.js b/app/assets/javascripts/discourse/app/widgets/widget.js index d3eae837cd7..1d7f4309455 100644 --- a/app/assets/javascripts/discourse/app/widgets/widget.js +++ b/app/assets/javascripts/discourse/app/widgets/widget.js @@ -146,7 +146,7 @@ export default class Widget { this.capabilities = register.lookup("capabilities:main"); this.store = register.lookup("service:store"); this.appEvents = register.lookup("service:app-events"); - this.keyValueStore = register.lookup("key-value-store:main"); + this.keyValueStore = register.lookup("service:key-value-store"); // We can inject services into widgets by passing a `services` parameter on creation (this.services || []).forEach((s) => { diff --git a/app/assets/javascripts/discourse/tests/helpers/create-store.js b/app/assets/javascripts/discourse/tests/helpers/create-store.js index aa224971c1b..4bf7c07189b 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-store.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-store.js @@ -65,7 +65,7 @@ export default function (customLookup = () => {}) { this._topicListAdapter || TopicListAdapter.create({ owner: this }); return this._topicListAdapter; } - if (type === "key-value-store:main") { + if (type === "service:key-value-store") { this._kvs = this._kvs || new KeyValueStore(); return this._kvs; }