From 056898c55f95b27e968d6ce44f12edc7651c3dff Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 27 Nov 2023 13:50:25 +0000 Subject: [PATCH] DEV: Prepare modal implementation for Ember upgrade (#24564) - Skip rendering DModalLegacy when running Ember 5 - Move named outlet inside the DModalLegacy component file - Exclude that DModalLegacy template from the build when running Ember 5 - Skip LegacySupport version of modal service when running Ember 5 - Add error popup for legacy modals when running Ember 5 Extracted from https://github.com/discourse/discourse/pull/21720. This is a no-op under our current Ember 3.28 version. --- .../app/components/d-modal-legacy.hbs | 2 +- .../app/components/modal-container.hbs | 44 +++++++++---------- .../app/components/modal-container.js | 5 +++ .../discourse/app/lib/ember-version.js | 5 +++ .../discourse/app/services/modal.js | 28 +++++++++++- .../javascripts/discourse/ember-cli-build.js | 23 +++++++++- 6 files changed, 79 insertions(+), 28 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/lib/ember-version.js diff --git a/app/assets/javascripts/discourse/app/components/d-modal-legacy.hbs b/app/assets/javascripts/discourse/app/components/d-modal-legacy.hbs index 725ae76127f..c1dfef988b1 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal-legacy.hbs +++ b/app/assets/javascripts/discourse/app/components/d-modal-legacy.hbs @@ -76,7 +76,7 @@ {{~this.flash.text~}} - {{yield}} + {{outlet "modalBody"}} {{#each this.errors as |error|}}
diff --git a/app/assets/javascripts/discourse/app/components/modal-container.hbs b/app/assets/javascripts/discourse/app/components/modal-container.hbs index 24ad1d2563b..8fbc159fc07 100644 --- a/app/assets/javascripts/discourse/app/components/modal-container.hbs +++ b/app/assets/javascripts/discourse/app/components/modal-container.hbs @@ -11,26 +11,24 @@ {{/each}} {{/if}} -{{! Legacy modals depend on this wrapper being in the DOM at all times. Eventually this will be dropped. -For now, we mitigate the potential impact on things like tests by removing the `modal` and `d-modal` classes when inactive }} - - {{outlet "modalBody"}} - \ No newline at end of file +{{#if this.renderLegacy}} + +{{/if}} \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/modal-container.js b/app/assets/javascripts/discourse/app/components/modal-container.js index a9fb2bd052e..7078b4936db 100644 --- a/app/assets/javascripts/discourse/app/components/modal-container.js +++ b/app/assets/javascripts/discourse/app/components/modal-container.js @@ -1,6 +1,7 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; import { inject as service } from "@ember/service"; +import { EMBER_MAJOR_VERSION } from "discourse/lib/ember-version"; export default class ModalContainer extends Component { @service modal; @@ -9,4 +10,8 @@ export default class ModalContainer extends Component { closeModal(data) { this.modal.close(data); } + + get renderLegacy() { + return EMBER_MAJOR_VERSION < 4; + } } diff --git a/app/assets/javascripts/discourse/app/lib/ember-version.js b/app/assets/javascripts/discourse/app/lib/ember-version.js new file mode 100644 index 00000000000..cde7ff34043 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/ember-version.js @@ -0,0 +1,5 @@ +import { VERSION } from "@ember/version"; + +const parts = VERSION.split("."); + +export const EMBER_MAJOR_VERSION = parseInt(parts[0], 10); diff --git a/app/assets/javascripts/discourse/app/services/modal.js b/app/assets/javascripts/discourse/app/services/modal.js index 19c2c7777b6..70df71c401e 100644 --- a/app/assets/javascripts/discourse/app/services/modal.js +++ b/app/assets/javascripts/discourse/app/services/modal.js @@ -5,6 +5,7 @@ import Service, { inject as service } from "@ember/service"; import { dasherize } from "@ember/string"; import $ from "jquery"; import { CLOSE_INITIATED_BY_MODAL_SHOW } from "discourse/components/d-modal"; +import { EMBER_MAJOR_VERSION } from "discourse/lib/ember-version"; import { disableImplicitInjections } from "discourse/lib/implicit-injections"; import deprecated, { withSilencedDeprecations, @@ -23,6 +24,8 @@ const LEGACY_OPTS = new Set([ @disableImplicitInjections class ModalService extends Service { + @service dialog; + @tracked activeModal; @tracked opts = {}; @@ -43,6 +46,23 @@ class ModalService extends Service { * @returns {Promise} A promise that resolves when the modal is closed, with any data passed to closeModal */ show(modal, opts) { + if (typeof modal === "string") { + this.dialog.alert( + `Error: the '${modal}' modal needs updating to work with the latest version of Discourse. See https://meta.discourse.org/t/268057.` + ); + deprecated( + `Defining modals using a controller is no longer supported. Use the component-based API instead. (modal: ${modal})`, + { + id: "discourse.modal-controllers", + since: "3.1", + dropFrom: "3.2", + url: "https://meta.discourse.org/t/268057", + raiseError: true, + } + ); + return; + } + this.close({ initiatedBy: CLOSE_INITIATED_BY_MODAL_SHOW }); let resolveShowPromise; @@ -50,7 +70,7 @@ class ModalService extends Service { resolveShowPromise = resolve; }); - this.opts = opts || {}; + this.opts = opts ??= {}; this.activeModal = { component: modal, opts, resolveShowPromise }; const unsupportedOpts = Object.keys(opts).filter((key) => @@ -75,7 +95,7 @@ class ModalService extends Service { } // Remove all logic below when legacy modals are dropped (deprecation: discourse.modal-controllers) -export default class ModalServiceWithLegacySupport extends ModalService { +class ModalServiceWithLegacySupport extends ModalService { @service appEvents; @tracked name; @@ -261,3 +281,7 @@ export default class ModalServiceWithLegacySupport extends ModalService { return this.name && !this.activeModal; } } + +export default EMBER_MAJOR_VERSION >= 4 + ? ModalService + : ModalServiceWithLegacySupport; diff --git a/app/assets/javascripts/discourse/ember-cli-build.js b/app/assets/javascripts/discourse/ember-cli-build.js index 85b91f46959..c7667f23dd1 100644 --- a/app/assets/javascripts/discourse/ember-cli-build.js +++ b/app/assets/javascripts/discourse/ember-cli-build.js @@ -17,8 +17,25 @@ const { StatsWriterPlugin } = require("webpack-stats-plugin"); const withSideWatch = require("./lib/with-side-watch"); const RawHandlebarsCompiler = require("discourse-hbr/raw-handlebars-compiler"); +const EMBER_MAJOR_VERSION = parseInt( + require("ember-source/package.json").version.split(".")[0], + 10 +); + process.env.BROCCOLI_ENABLED_MEMOIZE = true; +function filterForEmberVersion(tree) { + if (EMBER_MAJOR_VERSION < 4) { + return tree; + } + + return funnel(tree, { + // d-modal-legacy includes a named outlet which would cause + // a build failure in modern Ember + exclude: ["**/components/d-modal-legacy.hbs"], + }); +} + module.exports = function (defaults) { const discourseRoot = path.resolve("../../../.."); const vendorJs = discourseRoot + "/vendor/assets/javascripts/"; @@ -74,8 +91,10 @@ module.exports = function (defaults) { }, trees: { - app: RawHandlebarsCompiler( - withSideWatch("app", { watching: ["../discourse-markdown-it"] }) + app: filterForEmberVersion( + RawHandlebarsCompiler( + withSideWatch("app", { watching: ["../discourse-markdown-it"] }) + ) ), }, });