ReviewOS

stacks/stx

codemod: the tooltip rule rewrites a component `title` prop, emitting `x-tooltip` on a component (the #1830 construct) and duplicating the heading text

#1902
Closed glennmichael123 opened this 10 days ago ·
10 days ago

The tooltip rule treats every title="…" as an HTML attribute. On a component invocation, title is usually a prop, and --fix rewrites it anyway — producing x-tooltip on a component, which is the exact construct #1830 was filed about.

Repro

A component that declares title as a prop and renders it as heading text:

<!-- ValueCard.stx -->
<script server>
interface ValueCardProps {
  icon: string
  title: string
  description: string
}
const { icon, title, description } = withDefaults(defineProps<ValueCardProps>(), {})
</script>

<template>
  <div class="p-6">
    <h3 class="mt-5 font-semibold">{{ title }}</h3>
    <p class="mt-2 text-sm">{{ description }}</p>
  </div>
</template>

There is no title= attribute anywhere in that component.

Call site before:

<ValueCard icon="source-code" title="MIT licensed" description="Read every line, fork it, and ship your own." />

After stx codemod --fix:

<ValueCard icon="source-code" x-tooltip="MIT licensed" title="MIT licensed" description="Read every line, fork it, and ship your own." />

Why this is wrong three times over

  1. The rule's stated premise does not hold. The docblock justifies keeping title because "title= is announced by screen readers" and dropping it would "remove the accessible description". Here title is a prop rendered into an <h3> — there is no native tooltip and no accessible description to preserve. Nothing is being migrated; a second thing is being invented.

  2. It produces the construct from #1830 — x-tooltip on a component, rewritten to :tooltip and evaluated as an expression. The autofix now generates it.

  3. The result duplicates visible text. The card's own heading reappears as a tooltip on hover.

Scale

In one app, 3 of 14 tooltip findings were component props (<ValueCard title=…>); the other 11 were genuine <a>/<button> elements where the rule is correct and useful. So this is not "mute the rule" — it is one missing guard.

Suggested fix

Skip the rewrite when the tag name is a component rather than a known HTML element — the capitalised-first-letter convention already distinguishes them, same test the compiler uses to resolve <ValueCard> against a component file. Lowercase tags keep current behaviour.

Reporting these sites is still reasonable; it is only --fix that should hold off, in the same spirit as the confirm rule's "needs a human" bucket.

9 days ago

Fixed in 1ea9808b44.

Capitalised tags are now reported rather than rewritten. Your <ValueCard title=…> comes back untouched with a reason naming it as a component prop.

Reported rather than skipped, because a component may legitimately be forwarding the prop to a real element and a human can tell — the same "needs a human" bucket the confirm rule uses, as you suggested.

Your point 1 is the one that decided the shape of the fix: the rule's justification for keeping title is about the HTML attribute and the accessible description it carries. On a component prop there is no native tooltip and no accessible description to preserve, so the rewrite was not migrating anything — it was inventing a second thing and duplicating the card's own heading.

And the 3-of-14 framing was the right call: the other 11 were genuine elements where the rule is useful, so this is one missing guard rather than a reason to drop it. Lowercase tags and hyphenated custom elements (<my-widget title=…>, a real DOM element with a real title attribute) are unchanged, both pinned.

3 tests; removing the guard fails the first.

Sign in to comment on this issue.