From 89df65e8434ce18c88c0c4e1cc91eaef2d1e4137 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 20 Jan 2025 15:16:53 +0000 Subject: [PATCH] FIX: Avoid possible rendering infinite-loop in post-menu (#30873) ff815384 introduced a modifier which changes tracked state. If the conditions are correct, this can cause an infinite re-rendering loop. One example is [here](https://meta.discourse.org/t/346215/4), although there are other non-dev-tools things which could trigger this kind of loop. As a general rule, modifiers should not change tracked state. This commit changes the approach to match the rest of the new-post-menu assumptions: instead of trying to modify `collapsed` at runtime, the rendering of individual buttons has the `>1` logic. That matches the existing logic [here](https://github.com/discourse/discourse/blob/89ff7d51e6/app/assets/javascripts/discourse/app/components/post/menu.gjs#L392C18-L394C6). --- .../discourse/app/components/post/menu.gjs | 17 +++++++---------- .../components/post/menu/buttons/show-more.gjs | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/post/menu.gjs b/app/assets/javascripts/discourse/app/components/post/menu.gjs index ed70c9cc0f2..44883fd886d 100644 --- a/app/assets/javascripts/discourse/app/components/post/menu.gjs +++ b/app/assets/javascripts/discourse/app/components/post/menu.gjs @@ -3,7 +3,6 @@ import { cached, tracked } from "@glimmer/tracking"; import { hash } from "@ember/helper"; import { action } from "@ember/object"; import { getOwner } from "@ember/owner"; -import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import { service } from "@ember/service"; import { isEmpty, isPresent } from "@ember/utils"; import { and, eq } from "truth-helpers"; @@ -396,11 +395,6 @@ export default class PostMenu extends Component { return items; } - @action - expandIfOnlyOneHiddenButton() { - this.collapsed = this.renderableCollapsedButtons.length > 1; - } - @cached get renderableCollapsedButtons() { return this.availableCollapsedButtons.filter((button) => @@ -410,9 +404,13 @@ export default class PostMenu extends Component { @cached get visibleButtons() { - const nonCollapsed = this.availableButtons.filter((button) => { - return !this.availableCollapsedButtons.includes(button); - }); + let nonCollapsed = this.availableButtons; + + if (this.renderableCollapsedButtons.length > 1) { + nonCollapsed = nonCollapsed.filter((button) => { + return !this.renderableCollapsedButtons.includes(button); + }); + } return DAG.from( nonCollapsed.map((button) => [button.key, button, button.position]), @@ -609,7 +607,6 @@ export default class PostMenu extends Component { "replies-button-visible" ) }} - {{didInsert this.expandIfOnlyOneHiddenButton}} > {{! do not include PluginOutlets here, use the PostMenu DAG API instead }} {{#each this.extraControls key="key" as |extraControl|}} diff --git a/app/assets/javascripts/discourse/app/components/post/menu/buttons/show-more.gjs b/app/assets/javascripts/discourse/app/components/post/menu/buttons/show-more.gjs index d4aaa78c650..0fb6c4476e6 100644 --- a/app/assets/javascripts/discourse/app/components/post/menu/buttons/show-more.gjs +++ b/app/assets/javascripts/discourse/app/components/post/menu/buttons/show-more.gjs @@ -3,7 +3,7 @@ import DButton from "discourse/components/d-button"; export default class PostMenuShowMoreButton extends Component { static shouldRender(args) { - return args.state.collapsedButtons.length && args.state.collapsed; + return args.state.collapsedButtons.length > 1 && args.state.collapsed; }