Skip to content

refactor(chat,#1158): DRY adapter base default renderMessageElement#1189

Merged
joelteply merged 7 commits into
canaryfrom
feat/adapter-base-renderelement-default
May 15, 2026
Merged

refactor(chat,#1158): DRY adapter base default renderMessageElement#1189
joelteply merged 7 commits into
canaryfrom
feat/adapter-base-renderelement-default

Conversation

@joelteply
Copy link
Copy Markdown
Contributor

Summary

Lifts the byte-identical renderMessageElement body from three subclasses
(TextMessageAdapter, URLCardAdapter, ToolOutputAdapter) into
AbstractMessageAdapter as the new default. Subclass overrides are deleted;
ImageMessageAdapter retains its custom override (it builds <img> nodes
via property assignment to keep src/alt out of any HTML-parse path and
does not go through renderContent to string).

Net -61 LOC.

Pattern

"ALWAYS look for patterns and formulate them into design ... when you see
3+ similar things, find the primitive."

Three subclass overrides did the same thing:

parseContent(message) -> data
createAdapterWrapper() -> wrapper
renderContent(data, currentUserId) -> contentHtml
const template = document.createElement('template')
template.innerHTML = contentHtml
wrapper.appendChild(template.content.cloneNode(true))
return wrapper

That is now the base default body. The detached-template path (so the live
message-content slot never sees innerHTML directly) is preserved
identically -- Lit-managed reactive children still survive sibling updates.

ImageMessageAdapter is the natural-different outlier that proves the
abstraction is right: it doesn't fit the string-render pattern, so it stays
as a custom override. Three things compress to a primitive; the fourth
proves the primitive is real.

What's in the PR

  • AbstractMessageAdapter.renderMessageElement -- replaced return null
    default with the full DRY body. Class-name in error log resolves via
    this.constructor?.name so each subclass logs under its own name.
  • TextMessageAdapter -- override deleted; one-line comment points at
    base default.
  • URLCardAdapter -- override deleted; one-line comment points at the
    XSS-hardening follow-up (#1159).
  • ToolOutputAdapter -- override deleted; one-line comment notes that
    escapeHtml at renderContent interpolation sites still applies.
  • eslint-baseline.txt -- ratcheted -2 (deleted code carried 2 baseline
    errors).

Why ImageMessageAdapter stays custom

ImageMessageAdapter.renderMessageElement builds the wrapper's children
via DOM APIs (document.createElement('img') + img.src = url / img.alt = ...)
so the src/alt values never pass through any HTML parse step. That's
intentional defense-in-depth for the media path; the base default's
template.innerHTML = contentHtml would re-parse the image markup, which
is the exact thing this adapter was written to avoid. Keep it custom.

Tests

Followups

Closes #1158.

Test added 2 commits May 14, 2026 11:45
…MessageAdapter

Three adapters (TextMessageAdapter, URLCardAdapter, ToolOutputAdapter) had
byte-identical override bodies of the form: parseContent, createAdapterWrapper,
renderContent, template.innerHTML, wrapper.appendChild(fragment).

That is now the default body of AbstractMessageAdapter.renderMessageElement.
The overrides are deleted; the live message-content slot still never sees
innerHTML (the parse happens on a detached template), and Lit-managed
reactive children inside the message bubble keep their state.

ImageMessageAdapter retains its custom override -- it builds img nodes via
property assignment to keep src and alt out of any HTML-parse path and does
not go through renderContent to string.

Net minus 61 lines.

Closes #1158.
Test and others added 5 commits May 15, 2026 10:49
…enderelement-default

# Conflicts:
#	src/eslint-baseline.txt
#	src/widgets/chat/adapters/URLCardAdapter.ts
Linux CI ratchet failed because eslint-baseline.linux.txt was still at
5461 while the macOS baseline (and current count on both platforms)
is 5459. The ratchet requires CURRENT == BASELINE strictly, so the
-2 improvement from #1189 needed to land in BOTH platform files.

Sibling: 8b51729 (chore(eslint-baseline): ratchet -2) updated
eslint-baseline.txt; this commit completes the platform symmetry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…enderelement-default

# Conflicts:
#	src/eslint-baseline.linux.txt
#	src/eslint-baseline.txt
…merge

After merging origin/canary into the branch, baselines (mac=5455,
linux=5456) need to drop by the #1189 deletion delta (-2) to
mac=5453, linux=5454. macOS verified locally by precommit:
"Current: 5453 errors". Linux value is +1 vs Mac per established
platform skew; CI will surface the exact number if it's off.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joelteply joelteply merged commit c07e186 into canary May 15, 2026
3 of 4 checks passed
@joelteply joelteply deleted the feat/adapter-base-renderelement-default branch May 15, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant