feat: De-duplicate client console messages #4177
Conversation
gadenbuie
force-pushed
the
fix/error-console-dedupe-messages
branch
from
39c5221 to
6d33a3b
Compare
|
I'm fine with this PR's behavior as-is, but if the error console becomes more general in the future we should have a duplicate count like JS consoles have. |
srcts/src/components/errorConsole.ts
Outdated
| @@ -246,7 +270,7 @@ class ShinyErrorConsole extends LitElement { | |||
| </svg> | |||
| </button> | |||
| </div> | |||
| <slot class="content"></slot>`; | |||
| <slot class="content" @slotchange=${this.dedupeConsoleMessages}></slot>`; | |||
There was a problem hiding this comment.
I don't love that this dedupes after the content has changed, by deduping all of the messages. Could you instead change showShinyClientMessage so that right before performing the append, see if the message already exists? (Or have showShinyClientMessage call a method on ShinyErrorConsole that tracks all of this)
There was a problem hiding this comment.
I updated this to use a static method ShinyErrorConsole.appendConsoleMessage() to handle adding the message, skipping appending if the message already exists. 48d07c5
I agree and would like this too. As it is now, however, there's very little added value in showing message counts. It's more likely that we'd emit multiple messages signaling one problem than have multiple problems emit identical messages. The good news is that we still emit all messages to the browser console, where the call stack can help differentiate. If we make the console more general (which I'd like to see), we'll have to revisit this whole code path and I doubt we'll forget about adding message counts. |
srcts/src/components/errorConsole.ts
Outdated
| return msg; | ||
| } | ||
|
|
||
| static appendConsoleMessage( |
There was a problem hiding this comment.
Why static? Other than that, LGTM
There was a problem hiding this comment.
Good call, fixed in 2661a6f
Seen in rstudio/bslib#1159. It's possible for
bindAll()to be called in a way that we repeatedly issue the same warnings/errors for duplicate or repeated IDs.This PR updates the
shiny-error-consolecomponent to de-duplicate message when new messages are added to the console.