From df5561d780699541e2278c7160a60c662ed99a97 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 9 Aug 2024 15:04:05 +0100 Subject: [PATCH] DEV: Ensure deprecation warning banner works in development builds (#28302) In development, Ember raises an error when previously-used values are updated during a render. This is to avoid 'backtracking', where parts of templates have to be re-rendered multiple times. In general, this kind of pattern should be avoided, and Ember's warning helps us do that. However, for the deprecation warning banner, it is quite reasonable for some rendering to trigger a deprecation, and thereby require the global-notice to be re-rendered. We can use our `DeferredTrackedSet` to achieve that. Its `.add` method will delay adding an item to the Set until after the current render has completed. --- .../discourse/app/components/global-notice.js | 8 +++--- spec/system/ember_deprecation_spec.rb | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/global-notice.js b/app/assets/javascripts/discourse/app/components/global-notice.js index 1393d619f65..548f255fe00 100644 --- a/app/assets/javascripts/discourse/app/components/global-notice.js +++ b/app/assets/javascripts/discourse/app/components/global-notice.js @@ -2,16 +2,16 @@ import Component from "@ember/component"; import EmberObject, { action } from "@ember/object"; import { service } from "@ember/service"; import { htmlSafe } from "@ember/template"; -import { TrackedArray } from "@ember-compat/tracked-built-ins"; import { tagName } from "@ember-decorators/component"; import cookie, { removeCookie } from "discourse/lib/cookie"; +import { DeferredTrackedSet } from "discourse/lib/tracked-tools"; import { bind } from "discourse-common/utils/decorators"; import I18n from "discourse-i18n"; -const _pluginNotices = new TrackedArray(); +const _pluginNotices = new DeferredTrackedSet(); export function addGlobalNotice(text, id, options = {}) { - _pluginNotices.push(Notice.create({ text, id, options })); + _pluginNotices.add(Notice.create({ text, id, options })); } const GLOBAL_NOTICE_DISMISSED_PROMPT_KEY = "dismissed-global-notice-v2"; @@ -153,7 +153,7 @@ export default class GlobalNotice extends Component { notices.push(this.get("logNotice")); } - return notices.concat(_pluginNotices).filter((notice) => { + return notices.concat(Array.from(_pluginNotices)).filter((notice) => { if (notice.options.visibility) { return notice.options.visibility(notice); } diff --git a/spec/system/ember_deprecation_spec.rb b/spec/system/ember_deprecation_spec.rb index 738d4c642b7..a7c4fcc24a5 100644 --- a/spec/system/ember_deprecation_spec.rb +++ b/spec/system/ember_deprecation_spec.rb @@ -52,4 +52,30 @@ describe "JS Deprecation Handling", type: :system do ) expect(message).to have_text(SiteSetting.warn_critical_js_deprecations_message) end + + it "can show warnings triggered during initial render" do + sign_in Fabricate(:admin) + + t = Fabricate(:theme, name: "Theme With Tests") + t.set_field( + target: :extra_js, + type: :js, + name: "discourse/connectors/below-footer/my-connector.gjs", + value: <<~JS, + import deprecated from "discourse-common/lib/deprecated"; + function triggerDeprecation(){ + deprecated("Fake deprecation message", { id: "fake-deprecation" }) + } + export default + JS + ) + t.save! + SiteSetting.default_theme_id = t.id + + visit "/latest" + + expect(page).to have_css("#global-notice-critical-deprecation") + end end