Skip to content

prop-flow: resolve JSX spreads instead of reporting them as manual #933

Description

@fxOne

findAttr reports any {...spread} on a JSX element as unresolvable, which turns
the whole prop into an ambiguous site and the verdict into manual.

The cost is per occurrence, not per count: a spread anywhere in a pass-through
chain suppresses the verdict for that call site and for every leaf below it,
because the climb stops there. A single unresolved spread can be the difference
between a usable verdict and manual for a prop with a dozen call sites.

For calibration, an AST count over the ui project (JSX spread attributes only):

files onto Component tag onto intrinsic tag
first-party 14 7 7
shadcn (components/ui/) 25 77 22

Spreads onto intrinsic tags (<div {...props} />) carry no symbol, never enter
the usage index, and are irrelevant here. The shadcn block is vendored-but-
first-party source and mostly affects analysis of the library's internal
composition rather than application call sites. So the volume in everyday use is
modest — this change is justified by the blast radius per spread, not by how
many there are.

Most spreads are provably irrelevant for the prop under analysis, and the two
most common forms are fully resolvable with machinery that already exists.

Approach

Step 1 — exclude by type. Ask the checker whether the spread can carry the
prop at all:

const type = checker.getTypeAtLocation(spread.expression);
const sym = type.getProperty(propName);

undefined (and no index signature / any / unknown) means the spread cannot
supply this prop. Ignore the spread and fall through to the normal
present/omitted classification. Since we analyse one prop at a time and a spread
typically carries twenty others, this should resolve the majority of sites.

Step 2 — resolve what is left, in three tiers:

  1. {...props} — the enclosing props parameter. asEnclosingProp already
    recognises the identifier; the result is a passthrough to
    Enclosing.<propName> (same name, no rename) and feeds the existing
    recursion including the cycle guard.
  2. {...rest} — a rest element in the props destructure. TS types the rest
    binding as "props minus the destructured keys", so step 1 already does the
    right thing: a prop listed explicitly in the destructure is not in rest
    (spread irrelevant), anything else is a passthrough to
    Enclosing.<propName>. This also removes the current
    'rest element in props destructure' manual path in asEnclosingProp.
  3. Object literals and as const constants — look the key up and classify the
    value expression through classifyAttrValue.

An optional property in the spread type is not a blocker for tiers 1 and 2:
whether it is ever actually fed is precisely what the climb answers.

Stays manual

  • Opaque expressions ({...getProps()}): manual, whether the property is
    required or optional. The value is unknown either way, and a positive verdict
    must never be derived from an expression the analysis cannot see into — a
    real here would feed a wrong justified / unnecessary-optional.
  • any / unknown / index signatures (Record<string, unknown>) — must be
    detected explicitly, otherwise getProperty returning undefined produces a
    false negative.
  • Union types where only some constituents carry the prop.
  • Uninstantiated generics (spread inside a generic wrapper, type stays T).

Attribute precedence bug (fold in here)

findAttr returns on the first name match and only consults sawSpread on the
not-found path. <C {...props} title="x" /> is handled correctly (last
attribute wins), but <C title="x" {...props} /> is wrong — the spread
overrides the explicit attribute and the tool still reports "x".

The fix comes with the refactor: AttrLookup changes from the 'spread'
sentinel union to something like
{ attr: JsxAttribute | null; spreadsAfter: JsxSpreadAttribute[] }, which makes
ordering explicit. Multiple spreads: the last candidate that can supply the prop
wins; if an earlier one could also supply it and any candidate is
optional-or-opaque, fall back to manual.

Acceptance criteria

  • A spread whose type cannot carry the prop no longer produces an
    ambiguous site
  • <C {...props} /> resolves to a passthrough and climbs
  • <C {...rest} /> resolves, both for props inside and outside the
    destructure
  • any / index-signature spreads stay manual
  • <C title="x" {...props} /> is classified by the spread, not by the
    attribute
  • README limitations section updated

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions