ReviewOS

stacks/stx

typecheck parses `x-tooltip` as an expression, so `codemod --fix` produces markup that `stx typecheck` rejects (0 → 34 errors in one app)

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

stx codemod --fix adds x-tooltip. stx typecheck then reports the tooltip text as broken TypeScript. Running one shipped tool over an app makes the other shipped tool fail.

Repro

Before — a plain <a> with a title:

<a data-cal-day :href="cell.scheduleUrl" title="Schedule a post on this day">{{ cell.day }}</a>
$ stx typecheck "resources/views/queue.stx"
  Checked 1 script block(s) and 56 template expression(s) in 1 file(s) — 0 error(s), 0 warning(s)

Apply the tooltip rule:

$ stx codemod --fix "resources/views/queue.stx"
Rewrote 1 site(s):
  queue.stx:104  [tooltip]
<a data-cal-day :href="cell.scheduleUrl" x-tooltip="Schedule a post on this day" title="Schedule a post on this day">{{ cell.day }}</a>
$ stx typecheck "resources/views/queue.stx"
queue.stx:104:182  error TS1005  ')' expected.                     [template: Schedule a post on this day]
queue.stx:104:184  error TS1434  Unexpected keyword or identifier. [template: Schedule a post on this day]
queue.stx:104:189  error TS1434  Unexpected keyword or identifier. [template: Schedule a post on this day]
queue.stx:104:197  error TS1005  ';' expected.                     [template: Schedule a post on this day]
queue.stx:104:200  error TS1128  Declaration or statement expected.[template: Schedule a post on this day]
queue.stx:104:201  error TS1128  Declaration or statement expected.[template: Schedule a post on this day]

  Checked 1 script block(s) and 57 template expression(s) in 1 file(s) — 6 error(s), 0 warning(s)

56 expressions → 57, 0 errors → 6. One attribute, and it is being parsed as TypeScript.

Cause

NON_EXPRESSION_ATTRS in packages/stx/src/stx-virtual-ts.ts:373 — "Attributes that carry markup or a plain string, never an expression" — is missing x-tooltip:

const NON_EXPRESSION_ATTRS = new Set([
  'x-cloak',
  'x-ref',
  'x-transition',
  'x-teleport',
  'v-slot',
  'x-hydrate',
  ':key',
])

The runtime treats it as a plain string, not an expression — signals.ts:6050:

var text = el.getAttribute('x-tooltip');

x-tooltip-position (signals.ts:6055) is the same shape and also missing. Those are the only two: grepping getAttribute('x-…') across the runtime returns x-for, x-tooltip and x-tooltip-position, and x-for is a genuine expression already special-cased in FOR_ATTRS.

Impact

Any multi-word tooltip fails to parse. In one app, adopting the rule across 11 sites took the corpus from 0 template errors to 34 — all false, all produced by --fix.

It also lands on the two tools together: #1901 was a gate reporting errors that were not there, and the guidance in the sweep notes is that a gate which invents errors gets muted. This is the same outcome reached by a different route, except the app followed the tool's own advice to get there.

Fix

Add 'x-tooltip' and 'x-tooltip-position' to NON_EXPRESSION_ATTRS.

10 days ago

Fixed in 1911fbc404.

x-tooltip and x-tooltip-position are now in NON_EXPRESSION_ATTRS. Your repro is 0 errors again, and the expression count goes back to what it was — the attribute is no longer counted as one.

Confirmed against the runtime before choosing the fix rather than assuming: the tooltip block reads these with getAttribute('x-tooltip') and assigns straight to tip.textContent. It never evaluates them, so treating the value as an expression was simply wrong, and this is not a suppression.

The exemption is per attribute, not per element — an :href on the same tag is still checked. Pinned, because exempting the element would have been the easy version of this fix and would have quietly stopped checking real expressions.

"Running one shipped tool over an app makes the other shipped tool fail" is the right way to have framed it; that is what made this worth fixing ahead of the other two in the batch.

Sign in to comment on this issue.