diff --git a/packages/prop-flow/CHANGELOG.md b/packages/prop-flow/CHANGELOG.md
index 0b68496f..a167cdc0 100644
--- a/packages/prop-flow/CHANGELOG.md
+++ b/packages/prop-flow/CHANGELOG.md
@@ -5,6 +5,54 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## To Be Released
+## 3.1.0
+
+Three corrections to one failure: a prop that every caller passes, reported
+`caller-dead` — "inline the default, remove the prop" on live code. Two are
+holes in the walk; the third is the guard for whatever holes are left. Nothing
+in the library API or the CLI arguments moved, and `verdictOf` keeps its
+signature; `Site.kind` gains a value.
+
+- Fixed: `children` passed by JSX nesting was read as an omission. The value of
+ `children` is the one a call site writes *between* the tags rather than in the
+ attributes, and only the attributes were being read — so `
+ ` counted as a caller that passes nothing, and a `children?` nested at
+ every call site came back `caller-dead`. Nesting now wins over both an
+ attribute of that name and any spread, matching what JSX itself does;
+ `{slot}` is a pass-through like any other, while whitespace
+ between the tags, a lone `{/* comment */}` and `` correctly
+ reach nothing
+- Fixed: a component whose wrapper call only NAMES the function it wraps —
+ `function CardComponent(…) {}` plus `export const Card = memo(CardComponent)`,
+ the shape every wrapped component takes once it outgrows being written inline
+ — was two components to the walk. JSX renders `Card`, while a pass-through
+ climbing out of the body arrives at `CardComponent`, and the call sites filed
+ under the one name were invisible from the other. The climb ended on an empty
+ usage list, contributed nothing, and a prop that every caller passes came back
+ `caller-dead`: "delete the prop" on live code. The same gap made
+ `findComponents` skip such a file entirely, so pointing prop-flow at
+ `Card.tsx` reported no components at all. The wrapped function may live in
+ another file — `memo(CardImpl)` over an import resolves through the alias
+- A `caller-dead` whose sites include a pass-through that contributed nothing is
+ now reported `manual`. Both fixes above were one shape of the same failure:
+ the subtree came back a silent 0/0/0, the omissions written elsewhere were all
+ that was left, and the walk concluded nobody passes a prop that is passed on
+ every render. There are three reasons a subtree comes back empty — the
+ component really is dead, it is *called* rather than rendered (already
+ `manual`), or the walk filed its call sites under a different key. The first
+ and the third are indistinguishable, and only the first may safely end in
+ "delete this", so neither does. Scoped to `caller-dead` on purpose: under
+ `justified` or `unnecessary-optional` a silent pass-through changes nothing
+ anyone acts on, and downgrading those would throw away good verdicts to guard
+ against a risk that only exists where the advice is destructive
+- Such a pass-through is reported under a new `Site.kind`, `silent`, and the
+ hint names it, so the row says which line to go and check rather than leaving
+ the reader to notice that one of them is empty. The kind is printed under
+ every verdict, as evidence; only `caller-dead` is downgraded by it. The counts
+ never move — a downgraded row still reads `passes=0`, and what changed is the
+ conclusion drawn from it. A self-recursive pass-through is not a silence: the
+ first visit counted that subtree, and the repeat is meant to add nothing
+
## 3.0.0
- **BREAKING CHANGE**: a file with no exported component is no longer a failure.
diff --git a/packages/prop-flow/README.md b/packages/prop-flow/README.md
index 5dbdee99..7a29505e 100644
--- a/packages/prop-flow/README.md
+++ b/packages/prop-flow/README.md
@@ -105,7 +105,7 @@ cannot reach either.
| `unnecessary-optional` | every call site passes it → could be required |
| `caller-dead` | no call site passes it → optional and always `undefined` |
| `unused-component` | the component itself has no call sites in the Program |
-| `manual` | an unreadable spread or a contested override blocks a static conclusion |
+| `manual` | an unreadable spread, a contested override, or a pass-through that came back empty, blocks a static conclusion |
| `required` | the prop has no `?` to judge — listed only for its constant value |
Exit codes: `0` success, `1` nothing to do (usage printed), `2` a handled
@@ -214,22 +214,60 @@ ARIA props behind `React.ComponentProps<'button'>`, say — are not reported. A
verdict on them is true but useless: the `?` is not yours to drop, and they bury
the props that are. A prop redeclared in your own type is still reported.
-A pass-through that climbs into a function which is *called* rather than
-rendered — a `renderX({ … })` test helper, typically — also stays `manual`: its
-callers exist but are invisible to a JSX walk, and counting them as zero would
-report a live prop as `caller-dead`.
+### When a pass-through comes back empty
+
+A pass-through whose subtree moved no counter at all is reported as a `silent`
+site rather than a `passthrough`. There are three reasons a subtree comes back
+empty, and only one of them is an answer:
+
+1. **The component really is dead** — nothing renders it, nothing calls it.
+ Contributing nothing is the honest result.
+2. **The component is *called* rather than rendered** — a `renderX({ … })` test
+ helper. Its callers exist but are invisible to a JSX walk, so the site is
+ `manual` on its own, before any verdict is formed.
+3. **The walk filed its call sites under a different key.** A bug, by
+ definition — and not one you can see from the output.
+
+(1) and (3) are indistinguishable, so `caller-dead` — the one verdict whose
+advice is destructive — is not allowed to rest on either. **A `caller-dead`
+with at least one `silent` site is reported `manual` instead**, and the hint
+names the pass-through to go and check. The counts are untouched: the row still
+reads `passes=0`, and what changed is only the conclusion drawn from it.
+
+The guard is scoped to `caller-dead` on purpose. Under `justified` or
+`unnecessary-optional` a silent pass-through changes nothing anyone acts on, so
+those verdicts stand as they are — the `silent` line is still printed, as
+evidence rather than as a downgrade. In practice most `caller-dead` rows bottom
+out in direct omissions and carry no pass-through at all, so the rule is a
+no-op on them.
`prop={undefined}` counts as an omission — it is an omission dressed up as a
pass, so a prop that is only ever fed `undefined` still comes out as
`caller-dead`. A conditional expression that can evaluate to `undefined` counts
as a real source — the one false positive the tool accepts on purpose.
+`children` is read off the nesting, which is where JSX puts it rather than in
+the attributes. Nesting wins over an attribute of that name and over every
+spread, exactly as JSX resolves it: `{b}` passes
+`b`. A lone `{slot}` is a pass-through like any other value, so
+a forwarded `children` is traced to where it comes from. Whitespace between the
+tags, a lone `{/* comment */}` and `` reach nothing and stay
+omissions.
+
Components are picked up from `export function C`, `export const C = …`
(including `memo()` / `forwardRef()` wrappers), `export default function C` and
`export { C }` at the bottom of the file. A component re-exported through a
barrel is still found at its call sites, but must be inspected in the file that
declares it.
+A wrapper whose argument only *names* the function — `function CardComponent(…)
+{}` above `export const Card = memo(CardComponent)` — gives one component two
+names. Call sites are written as ``, while a pass-through climbing out of
+the body arrives at `CardComponent`; both reach the same entry, so a prop fed
+through the wrapper is counted at every call site rather than at none of them.
+The wrapped function may sit in another file — `memo(CardImpl)` over an import
+is followed through the alias.
+
An exported `useX` taking an options object is skipped. It is indistinguishable
from a component to the AST and has no JSX call sites, so every one of its
options would come back `unused-component` — a statement about the walk, not
diff --git a/packages/prop-flow/fixtures/basic/app.tsx b/packages/prop-flow/fixtures/basic/app.tsx
index 75cfeb2d..05ce9dbe 100644
--- a/packages/prop-flow/fixtures/basic/app.tsx
+++ b/packages/prop-flow/fixtures/basic/app.tsx
@@ -9,6 +9,7 @@ import { Panel } from './panel';
import { Renamed } from './renamed';
import { Rest } from './rest';
import { Tree } from './tree';
+import { Chrome, Remote, Stripe } from './wrapped';
const badgeProps: BadgeProps = { text: 'spread', tone: 'info' };
@@ -34,6 +35,12 @@ export function App() {
+
+
+ {/* The omission that makes Stripe.loud caller-dead as long as the walk
+ cannot see through to the function it wraps. */}
+
+
);
}
diff --git a/packages/prop-flow/fixtures/basic/children.tsx b/packages/prop-flow/fixtures/basic/children.tsx
new file mode 100644
index 00000000..ad7b0321
--- /dev/null
+++ b/packages/prop-flow/fixtures/basic/children.tsx
@@ -0,0 +1,80 @@
+// What reaches `children`, and what only looks like it does. Nesting is the one
+// prop value written outside the attributes, so every shape below is invisible
+// to a walk that reads attributes alone — and `children` is the prop where
+// concluding "nobody passes it" does the most damage.
+//
+// `ChildrenApp` at the bottom is the only render root in this file, so nothing
+// here moves the counts the other fixtures assert.
+
+export interface SlotProps {
+ /** justified: nested at most call sites, genuinely absent at others */
+ children?: JSX.Element;
+ id: string;
+}
+
+export function Slot({ children, id }: SlotProps) {
+ return {children}
;
+}
+
+export interface RelayedProps {
+ /** justified: forwarded by nesting, which is a pass-through like any other */
+ children?: JSX.Element;
+ id: string;
+}
+
+export function Relayed({ children, id }: RelayedProps) {
+ return {children};
+}
+
+export interface ShellProps {
+ /** unnecessary-optional: nested once, and never omitted */
+ children?: JSX.Element;
+ id: string;
+}
+
+/** Spreads its whole props object AND nests: the nesting is what arrives. */
+export function Shell(props: ShellProps) {
+ return
+
+ ;
+}
+
+export function ChildrenApp() {
+ return (
+
+ {/* Values that reach `children` */}
+
+
+
+ plain text
+ {'from an expression'}
+
+
+
+
+ {/* An attribute is still read where nothing is nested… */}
+ } />
+ {/* …and loses to the nesting where there is some. */}
+ }>
+
+
+
+ {/* Nestings that reach nothing */}
+
+
+
+
+
+ {undefined}
+
+ {/* Forwarded rather than originated */}
+
+
+
+
+
+
+ );
+}
diff --git a/packages/prop-flow/fixtures/basic/silent.tsx b/packages/prop-flow/fixtures/basic/silent.tsx
new file mode 100644
index 00000000..b3a7d5f7
--- /dev/null
+++ b/packages/prop-flow/fixtures/basic/silent.tsx
@@ -0,0 +1,45 @@
+// A `caller-dead` that a silent pass-through pulls back to MANUAL. `Base.note`
+// is omitted at every call site the walk can read, and the one remaining site
+// climbs into `Relayed`, which is rendered nowhere — so the subtree comes back
+// empty and the counts say "nobody passes it" on the strength of a hole.
+
+export interface BaseProps {
+ id: string;
+ /** manual: every readable caller omits it, one pass-through said nothing */
+ note?: string;
+}
+
+export function Base({ id, note }: BaseProps) {
+ return {note};
+}
+
+export interface RelayedProps {
+ id: string;
+ note?: string;
+}
+
+/** Exported and forwards `note`, but rendered nowhere in the Program. */
+export function Relayed(props: RelayedProps) {
+ return ;
+}
+
+export interface LoudProps {
+ id: string;
+ /** caller-dead: nothing but direct omissions — the guard is a no-op here */
+ note?: string;
+}
+
+export function Loud({ id, note }: LoudProps) {
+ return {note};
+}
+
+export function SilentApp() {
+ return (
+
+
+
+
+
+
+ );
+}
diff --git a/packages/prop-flow/fixtures/basic/wrapped-impl.tsx b/packages/prop-flow/fixtures/basic/wrapped-impl.tsx
new file mode 100644
index 00000000..8d1b9fa7
--- /dev/null
+++ b/packages/prop-flow/fixtures/basic/wrapped-impl.tsx
@@ -0,0 +1,22 @@
+export interface TintedProps {
+ /** unnecessary-optional, and only reachable across the wrapper AND the file
+ * boundary: the climb lands on RemoteImpl here, while every call site is a
+ * written against the binding wrapped.tsx exports. */
+ shade?: string;
+}
+
+export function Tinted({ shade }: TintedProps) {
+ return ;
+}
+
+export interface RemoteProps {
+ /** unnecessary-optional: the one call site passes it, through the wrapper */
+ tint?: string;
+}
+
+// The implementation half of a wrapper split across two files — wrapped.tsx
+// imports this name and hands it to memo(). Resolving the wrapper argument has
+// to follow the import alias, not just the local name.
+export function RemoteImpl({ tint }: RemoteProps) {
+ return ;
+}
diff --git a/packages/prop-flow/fixtures/basic/wrapped.tsx b/packages/prop-flow/fixtures/basic/wrapped.tsx
new file mode 100644
index 00000000..a428e365
--- /dev/null
+++ b/packages/prop-flow/fixtures/basic/wrapped.tsx
@@ -0,0 +1,41 @@
+import { RemoteImpl } from './wrapped-impl';
+
+// The wrapper shape panel.tsx does not cover: the wrapped function is
+// DECLARED, and the wrapper call only names it. That gives one component two
+// symbols — JSX renders `Chrome`, while a pass-through climbs into
+// `ChromeComponent` — and a walk that keeps them apart finds no call site on
+// the way up, which reads as "nobody passes this prop".
+
+export interface StripeProps {
+ /** justified: passed through Chrome, and omitted at a direct call site */
+ loud?: boolean;
+}
+
+export function Stripe({ loud }: StripeProps) {
+ return
;
+}
+
+export interface ChromeProps {
+ /** the prop the pass-through climbs to — declared on the inner function */
+ highlight?: boolean;
+}
+
+function ChromeComponent({ highlight }: ChromeProps) {
+ return ;
+}
+
+// Stand-in for React.memo, as in panel.tsx — here wrapping a name, not a
+// function written out inside the call.
+const memo = (component: T): T => component;
+
+export const Chrome = memo(ChromeComponent);
+
+// The same wrapper, but the name it hands to memo() is an IMPORT. Following it
+// means resolving the alias first — the local name resolves to nothing.
+export const Remote = memo(RemoteImpl);
+
+// Wrapper arguments that name no function: one that does not exist at all, and
+// one that names a type. Following a name must not turn either into a
+// component — nor stop the walk on the way past.
+export const Missing = memo(Absent);
+export const Typed = memo(ChromeProps);
diff --git a/packages/prop-flow/package.json b/packages/prop-flow/package.json
index 323566d0..932b1db7 100644
--- a/packages/prop-flow/package.json
+++ b/packages/prop-flow/package.json
@@ -1,6 +1,6 @@
{
"name": "@fxone/prop-flow",
- "version": "3.0.0",
+ "version": "3.1.0",
"description": "trace an optional prop across every JSX call site and tell whether its `?` is justified",
"keywords": [
"typescript",
diff --git a/packages/prop-flow/src/analyzer.test.ts b/packages/prop-flow/src/analyzer.test.ts
index eb762ef6..21af0f26 100644
--- a/packages/prop-flow/src/analyzer.test.ts
+++ b/packages/prop-flow/src/analyzer.test.ts
@@ -61,6 +61,9 @@ describe('createAnalyzer', () => {
['button.tsx', 'Button', 'size', 'unnecessary-optional'],
['button.tsx', 'Button', 'title', 'justified'],
['card.tsx', 'Card', 'action', 'justified'],
+ ['children.tsx', 'Relayed', 'children', 'justified'],
+ ['children.tsx', 'Shell', 'children', 'unnecessary-optional'],
+ ['children.tsx', 'Slot', 'children', 'justified'],
['dialog.tsx', 'Dialog', 'caption', 'justified'],
['frame.tsx', 'Frame', 'caption', 'justified'],
['frame.tsx', 'Frame', 'tone', 'caller-dead'],
@@ -71,10 +74,16 @@ describe('createAnalyzer', () => {
['panel.tsx', 'Panel', 'note', 'justified'],
['renamed.tsx', 'Renamed', 'caption', 'unnecessary-optional'],
['rest.tsx', 'Rest', 'extra', 'caller-dead'],
+ ['silent.tsx', 'Base', 'note', 'manual'],
+ ['silent.tsx', 'Loud', 'note', 'caller-dead'],
['sink.tsx', 'Sink', 'data', 'unnecessary-optional'],
['spread.tsx', 'Leaf', 'note', 'justified'],
['spread.tsx', 'Murky', 'note', 'manual'],
['tree.tsx', 'Tree', 'depth', 'unnecessary-optional'],
+ ['wrapped.tsx', 'Chrome', 'highlight', 'justified'],
+ ['wrapped.tsx', 'Remote', 'tint', 'unnecessary-optional'],
+ ['wrapped.tsx', 'Stripe', 'loud', 'justified'],
+ ['wrapped-impl.tsx', 'Tinted', 'shade', 'unnecessary-optional'],
])('%s: %s.%s → %s', (file, component, prop, verdict) => {
expect(analyse(file, component, prop).verdict).toBe(verdict);
});
@@ -127,10 +136,14 @@ describe('createAnalyzer', () => {
'passthrough ListConst.note',
'passthrough Multi.note',
'passthrough Override.note',
- // Never rendered and never called — the site below it is dead code.
- 'passthrough Unrendered.note',
'real string literal',
+ // Never rendered and never called — the site below it is dead code, so
+ // the subtree is empty and the line says so. It does not pull the verdict
+ // back: five other sites pass the prop, and the downgrade only guards the
+ // one verdict whose advice is to delete something.
+ 'silent Unrendered.note',
]);
+ expect(result.verdict).toBe('justified');
});
it('keeps the three spread shapes it must not resolve MANUAL', () => {
@@ -243,11 +256,96 @@ describe('createAnalyzer', () => {
]);
});
+ it('reads `children` off the nesting, which no attribute carries', () => {
+ // The prop whose value is written between the tags rather than in the
+ // attributes. Read attributes alone and every nesting looks like an
+ // omission — on `children`, of all props, that ends in "nobody passes it".
+ const result = analyse('children.tsx', 'Slot', 'children');
+
+ expect(result).toMatchObject({ ambiguous: 0, omit: 6, real: 8 });
+ expect(tally(result.sites)).toEqual({
+ // Self-closing, ``, whitespace between the tags, and a lone
+ // comment — four ways to nest nothing at all.
+ 'omit': 4,
+ 'omit explicit undefined': 1,
+ // Forwarded by nesting: a pass-through like any other.
+ 'passthrough Relayed.children': 1,
+ // An attribute is still read wherever nothing is nested.
+ 'real JsxSelfClosingElement': 1,
+ // An element, text, several children at once, `children={…}` beaten by a
+ // nesting, and the spread that the nesting beats too.
+ 'real nested children': 5,
+ // A lone `{expr}` is the one nesting whose value can be read.
+ 'real string literal': 1,
+ });
+ });
+
+ it('lets a nesting beat a spread that would otherwise carry children', () => {
+ // . The spread's type has `children`, so
+ // without the nesting this resolves to a pass-through of Shell's own — the
+ // nesting is what actually arrives, and it originates right here.
+ const result = analyse('children.tsx', 'Shell', 'children');
+
+ expect(result).toMatchObject({ ambiguous: 0, omit: 0, real: 1 });
+ expect(result.sites.map(describeSite)).toEqual(['real nested children']);
+ });
+
+ it('climbs out of a function its export wraps by name', () => {
+ // `export const Chrome = memo(ChromeComponent)`. The climb out of
+ // lands on ChromeComponent, whose call sites are
+ // written as — a different symbol. Filed apart, the pass-through
+ // contributes nothing, the lone in app.tsx is all that is left,
+ // and a prop that IS passed comes back caller-dead: "delete it" on live code.
+ const result = analyse('wrapped.tsx', 'Stripe', 'loud');
+
+ expect(result).toMatchObject({ ambiguous: 0, omit: 2, real: 1, verdict: 'justified' });
+ // The pass and the omission are both counted through
+ // the one pass-through line, as every expanded subtree is; the second omit
+ // is the direct . Reported under the name that declares the prop,
+ // not under the binding the wrapper call was assigned to.
+ expect(result.sites.map(describeSite).sort()).toEqual(['omit', 'passthrough ChromeComponent.highlight']);
+ });
+
+ it('climbs out of a wrapped function that lives in another file', () => {
+ // `export const Remote = memo(RemoteImpl)` in wrapped.tsx, RemoteImpl in
+ // wrapped-impl.tsx. The climb out of lands on
+ // RemoteImpl; every call site is a written elsewhere against the
+ // binding. Both halves of the fix have to hold at once — peel the wrapper
+ // AND follow the import alias — or this is a caller-dead again.
+ const result = analyse('wrapped-impl.tsx', 'Tinted', 'shade');
+
+ expect(result).toMatchObject({ ambiguous: 0, omit: 0, real: 1 });
+ expect(result.sites.map(describeSite)).toEqual(['passthrough RemoteImpl.tint']);
+ });
+
+ it('pulls a caller-dead back to manual when a pass-through said nothing', () => {
+ // Two direct omissions and one climb into , which is rendered
+ // nowhere — so the subtree moves no counter and the counts read "nobody
+ // passes it" on the strength of a hole. `caller-dead` is the one verdict
+ // whose advice is destructive, so it is the one an empty subtree must not
+ // be allowed to reach.
+ const result = analyse('silent.tsx', 'Base', 'note');
+
+ expect(result).toMatchObject({ ambiguous: 0, omit: 2, real: 0, verdict: 'manual' });
+ // The counts are untouched — only the verdict moves. What the row gains is
+ // the line naming where a caller could still be hiding.
+ expect(result.sites.map(describeSite).sort()).toEqual(['omit', 'omit', 'silent Relayed.note']);
+ expect(result.sites.find(({ kind }) => kind === 'silent')?.note).toBe('contributed nothing');
+ });
+
+ it('leaves a caller-dead alone when every site bottoms out directly', () => {
+ // The same file, the same two-omission shape, no pass-through anywhere:
+ // the guard must not cost the verdict its usual case.
+ expect(analyse('silent.tsx', 'Loud', 'note')).toMatchObject({ omit: 2, real: 0, verdict: 'caller-dead' });
+ });
+
it('terminates on a self-recursive component instead of looping', () => {
const result = analyse('tree.tsx', 'Tree', 'depth');
// The self-recursive call site is a pass-through back into Tree.depth: it
- // is reported, but the repeat visit contributes no counts.
+ // is reported, but the repeat visit contributes no counts. Moving no
+ // counter is what a silence looks like, and this is not one — the first
+ // visit already counted the subtree, so the line stays a plain pass-through.
expect(result).toMatchObject({ ambiguous: 0, omit: 0, real: 1 });
expect(result.sites.map(({ kind }) => kind).sort()).toEqual(['passthrough', 'real']);
expect(result.sites.find(({ kind }) => kind === 'passthrough')?.via).toBe('Tree.depth');
@@ -257,6 +355,16 @@ describe('createAnalyzer', () => {
expect(analyzer.findComponents(sourceFile('button.tsx')).map(({ name }) => name)).toEqual(['Button']);
// memo()-wrapped arrow, assigned to an exported const.
expect(analyzer.findComponents(sourceFile('panel.tsx')).map(({ name }) => name)).toEqual(['Panel']);
+ // The same wrapper over a function the call only NAMES: stopping at the
+ // identifier leaves the file looking like it exports no component at all.
+ // `Missing` and `Typed` wrap a name that resolves to nothing and one that
+ // resolves to a type — following a name must not invent a component either.
+ expect(analyzer.findComponents(sourceFile('wrapped.tsx')).map(({ name }) => name)).toEqual([
+ 'Stripe',
+ 'Chrome',
+ // Wrapped by a name that is an import: the alias resolves before the peel.
+ 'Remote',
+ ]);
// Exported at the bottom of the file — reported under their declared name.
expect(analyzer.findComponents(sourceFile('late.tsx')).map(({ name }) => name)).toEqual(['Aliased', 'Late']);
// App takes no props, so there is nothing to analyse in it.
diff --git a/packages/prop-flow/src/analyzer.ts b/packages/prop-flow/src/analyzer.ts
index d36ce381..c2472fa0 100644
--- a/packages/prop-flow/src/analyzer.ts
+++ b/packages/prop-flow/src/analyzer.ts
@@ -1,5 +1,5 @@
import type * as TS from 'typescript';
-import { defaultedBindings, isExported, unwrapToFn } from './ast.js';
+import { defaultedBindings, isExported } from './ast.js';
import { createClassifier } from './classify.js';
import { createComponentFactory } from './component.js';
import type { Component } from './component.js';
@@ -30,6 +30,13 @@ interface Contribution {
/** A value that could not be read as a literal — no constancy is claimable. */
readonly poisoned: boolean;
readonly real: number;
+ /**
+ * `Component.prop` of every pass-through below here whose subtree moved no
+ * counter at all. Carried rather than derived from `sites`, because expanding
+ * a pass-through replaces the subtree's lines with one of its own — a deeper
+ * silence would be gone from the sites by the time the verdict is decided.
+ */
+ readonly silent: readonly string[];
/** One line per call site, except where absorbing a default splits it up. */
readonly sites: readonly Site[];
readonly values: readonly string[];
@@ -44,6 +51,7 @@ interface Walk {
readonly omit: number;
readonly poisoned: boolean;
readonly real: number;
+ readonly silent: readonly string[];
readonly sites: readonly Site[];
readonly values: ReadonlySet;
readonly verdict: Verdict;
@@ -84,17 +92,45 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer
omit: result.omit,
real: result.real,
sites: result.sites,
- verdict: result.verdict,
+ verdict: downgraded(result),
};
}
+ /**
+ * `caller-dead` is the one verdict whose advice is destructive — "no caller
+ * passes it, remove the prop" — so it is the one that must not be reached by
+ * accident. A subtree that contributed nothing is indistinguishable from a
+ * subtree that legitimately had nothing to contribute, and only the second
+ * may end here: the first is a call site the walk could not see, and acting
+ * on it deletes a live prop.
+ *
+ * Scoped to `caller-dead` on purpose. Under `justified` or
+ * `unnecessary-optional` a silent pass-through changes nothing anyone acts
+ * on, and downgrading those would throw away good verdicts to guard against a
+ * risk that only exists where the advice is "delete this".
+ */
+ function downgraded(result: Walk): Verdict {
+ return result.verdict === 'caller-dead' && result.silent.length > 0 ? 'manual' : result.verdict;
+ }
+
/** Every call site of `component`, merged into counts, sites and values. */
function walk(component: Component, propName: string, visited: Set): Walk {
const key = `${index.symbolId(component.symbol)}#${propName}`;
if (visited.has(key)) {
// A cycle in the pass-through graph. The first visit already counted
- // this subtree, so the repeat contributes nothing.
- return { ambiguous: 0, omit: 0, poisoned: false, real: 0, sites: [], values: new Set(), verdict: 'cycle' };
+ // this subtree, so the repeat contributes nothing — deliberately, which
+ // is what keeps it out of `silent`: the information is in the result
+ // already, just reached by the other branch.
+ return {
+ ambiguous: 0,
+ omit: 0,
+ poisoned: false,
+ real: 0,
+ silent: [],
+ sites: [],
+ values: new Set(),
+ verdict: 'cycle',
+ };
}
visited.add(key);
@@ -142,14 +178,16 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer
const loc = locOf(el, cwd);
const trace = `${via.name}.${viaProp}`;
const fallback = defaultsOf(via).get(viaProp);
+ const silent = silentOf(sub, trace);
if (sub.omit === 0 || fallback === undefined) {
// No omission for the default to catch, or no default to catch it.
return {
+ silent,
ambiguous: sub.ambiguous,
omit: sub.omit,
poisoned: sub.poisoned,
real: sub.real,
- sites: [{ kind: 'passthrough', loc, via: trace }],
+ sites: [passthroughSite(loc, trace, silent)],
values: [...sub.values],
};
}
@@ -163,6 +201,9 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer
.filter((site) => site.kind === 'omit')
.map((site): Site => ({ kind: 'real', loc: site.loc, note: 'the default fires here', via: trace }));
return {
+ // Absorbing a default means the subtree had omissions, so this line is
+ // never the silent one — it just carries a deeper silence past itself.
+ silent,
ambiguous: sub.ambiguous,
omit: 0,
poisoned: sub.poisoned || value === null,
@@ -217,12 +258,12 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer
return [];
}
- /** The component `export const C = memo(() => …)` binds, if it binds one. */
+ /** The component `export const C = memo(…)` binds, if it binds one. */
function componentOfDeclaration(decl: TS.VariableDeclaration): Component | null {
if (!ts.isIdentifier(decl.name) || !decl.initializer) {
return null;
}
- const fn = unwrapToFn(ts, decl.initializer);
+ const fn = components.unwrap(decl.initializer);
return fn ? components.fromNode(fn, decl.name) : null;
}
@@ -277,11 +318,49 @@ function leaf(counts: Counts, site: Site, value: string | null = null): Contribu
omit: counts.omit ?? 0,
poisoned: site.kind === 'real' && value === null,
real: counts.real ?? 0,
+ // A leaf moves a counter by construction, so it is never a silence — and it
+ // has no subtree for one to arrive from.
+ silent: [],
sites: [site],
values: value === null ? [] : [value],
};
}
+/**
+ * The pass-throughs under this call site that moved no counter at all. A
+ * subtree that came back empty IS one, and it stands in for whatever silence
+ * lies below it: the deeper traces are dropped rather than added, because this
+ * is the line the reader can actually go and look at.
+ */
+function silentOf(sub: Walk, trace: string): readonly string[] {
+ if (sub.verdict === 'cycle') {
+ // Not a silence. The first visit counted this subtree and the repeat is
+ // meant to add nothing — the information is in the result already.
+ return [];
+ }
+ return sub.ambiguous === 0 && sub.omit === 0 && sub.real === 0 ? [trace] : sub.silent;
+}
+
+/**
+ * The one line a pass-through contributes. A silent one is named apart because
+ * it is indistinguishable from a resolved one on the page: same shape, same
+ * trace, and nothing underneath to show for it.
+ */
+function passthroughSite(loc: string, trace: string, silent: readonly string[]): Site {
+ if (silent.length === 0) {
+ return { kind: 'passthrough', loc, via: trace };
+ }
+ // `via` already names this pass-through, so a note repeating it reads as two
+ // traces. Only a silence further down has something left to say.
+ const deeper = silent.filter((name) => name !== trace);
+ return {
+ loc,
+ kind: 'silent',
+ note: deeper.length > 0 ? `${deeper.join(', ')} contributed nothing` : 'contributed nothing',
+ via: trace,
+ };
+}
+
/** Fold every call site's contribution into the walk's totals. */
function merge(contributions: readonly Contribution[]): Omit {
return {
@@ -289,6 +368,7 @@ function merge(contributions: readonly Contribution[]): Omit {
omit: total(contributions, 'omit'),
poisoned: contributions.some((contribution) => contribution.poisoned),
real: total(contributions, 'real'),
+ silent: contributions.flatMap((contribution) => contribution.silent),
sites: contributions.flatMap((contribution) => contribution.sites),
values: new Set(contributions.flatMap((contribution) => contribution.values)),
};
diff --git a/packages/prop-flow/src/ast.test.ts b/packages/prop-flow/src/ast.test.ts
index 3f7b721c..9d383231 100644
--- a/packages/prop-flow/src/ast.test.ts
+++ b/packages/prop-flow/src/ast.test.ts
@@ -5,12 +5,35 @@ import {
bindingNameOfFn,
defaultedBindings,
findAttr,
+ fnOfDeclaration,
isComponentFn,
isExported,
unwrapToFn,
} from './ast.js';
import type { ComponentFn } from './ast.js';
+/**
+ * The checker's job, faked over one parsed file: a name maps to the top-level
+ * function or variable that declares it. These fixtures are parsed, not
+ * compiled, so there is no checker to ask.
+ */
+function topLevelDeclaration(id: ts.Identifier): ts.Declaration | null {
+ for (const statement of id.getSourceFile().statements) {
+ if (ts.isFunctionDeclaration(statement) && statement.name?.text === id.text) {
+ return statement;
+ }
+ if (ts.isVariableStatement(statement)) {
+ const declared = statement.declarationList.declarations.find(
+ (decl) => ts.isIdentifier(decl.name) && decl.name.text === id.text,
+ );
+ if (declared) {
+ return declared;
+ }
+ }
+ }
+ return null;
+}
+
function parse(code: string, setParentNodes = true): ts.SourceFile {
return ts.createSourceFile('fixture.tsx', code, ts.ScriptTarget.ES2023, setParentNodes, ts.ScriptKind.TSX);
}
@@ -90,6 +113,42 @@ describe('unwrapToFn', () => {
it.each([['const C = memo();'], ['const C = somethingElse;']])('gives up on %s', (code) => {
expect(unwrapToFn(ts, initializerOf(code))).toBeNull();
});
+
+ it.each([
+ ['const C = memo(Inner); function Inner(p: P) {}', ts.SyntaxKind.FunctionDeclaration],
+ ['const C = memo(Inner); const Inner = memo((p: P) => null);', ts.SyntaxKind.ArrowFunction],
+ ])('follows a wrapped name into what it declares in %s', (code, kind) => {
+ expect(unwrapToFn(ts, initializerOf(code), topLevelDeclaration)?.kind).toBe(kind);
+ });
+
+ it('follows a name only when handed something to resolve it with', () => {
+ // Without a resolver this is where the peel stops — which is the whole
+ // reason a wrapped-by-name component used to be invisible.
+ expect(unwrapToFn(ts, initializerOf('const C = memo(Inner); function Inner(p: P) {}'))).toBeNull();
+ });
+
+ it('gives up on names that resolve back to each other', () => {
+ // Not runnable code, but it parses — and half-written files are exactly
+ // what this tool gets pointed at.
+ expect(unwrapToFn(ts, initializerOf('const A = B; const B = A;'), topLevelDeclaration)).toBeNull();
+ });
+});
+
+describe('fnOfDeclaration', () => {
+ it('reads the function a declaration is, or the one its initializer holds', () => {
+ expect(fnOfDeclaration(ts, findNode('function C(p: P) {}', ts.isFunctionDeclaration))?.kind).toBe(
+ ts.SyntaxKind.FunctionDeclaration,
+ );
+ expect(fnOfDeclaration(ts, findNode('const C = (p: P) => null;', ts.isVariableDeclaration))?.kind).toBe(
+ ts.SyntaxKind.ArrowFunction,
+ );
+ });
+
+ it('has nothing to read from a missing declaration or one holding no function', () => {
+ expect(fnOfDeclaration(ts, null)).toBeNull();
+ expect(fnOfDeclaration(ts, findNode('const C = 1;', ts.isVariableDeclaration))).toBeNull();
+ expect(fnOfDeclaration(ts, findNode('class C { render(p: P) {} }', ts.isClassDeclaration))).toBeNull();
+ });
});
describe('defaultedBindings', () => {
diff --git a/packages/prop-flow/src/ast.ts b/packages/prop-flow/src/ast.ts
index 8d0a9948..f9ee2382 100644
--- a/packages/prop-flow/src/ast.ts
+++ b/packages/prop-flow/src/ast.ts
@@ -23,14 +23,69 @@ export function isComponentFn(ts: TypeScriptApi, node: TS.Node): node is Compone
return ts.isArrowFunction(node) || ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node);
}
-/** Peel memo()/forwardRef()/React.memo() wrappers down to the inner function. */
-export function unwrapToFn(ts: TypeScriptApi, expr: TS.Expression): ComponentFn | null {
+/**
+ * How an identifier is resolved to what it declares. Optional wherever it is
+ * taken: the AST alone cannot follow a name, so a plain parse hands none.
+ */
+export type DeclarationResolver = (id: TS.Identifier) => TS.Declaration | null;
+
+/**
+ * Peel memo()/forwardRef()/React.memo() wrappers down to the inner function.
+ * Given a resolver, a wrapper argument that merely NAMES the function is
+ * followed too: `export const Card = memo(CardComponent)` is the shape a
+ * wrapped component takes as soon as it outgrows being written inline, and
+ * stopping at the identifier leaves the component undiscoverable.
+ */
+export function unwrapToFn(ts: TypeScriptApi, expr: TS.Expression, resolve?: DeclarationResolver): ComponentFn | null {
+ return peel(ts, expr, resolve, new Set());
+}
+
+/** The component function a declaration IS, or that its initializer peels to. */
+export function fnOfDeclaration(
+ ts: TypeScriptApi,
+ declaration: TS.Declaration | null | undefined,
+ resolve?: DeclarationResolver,
+): ComponentFn | null {
+ return declaration ? fnOfDecl(ts, declaration, resolve, new Set()) : null;
+}
+
+function peel(
+ ts: TypeScriptApi,
+ expr: TS.Expression,
+ resolve: DeclarationResolver | undefined,
+ seen: Set,
+): ComponentFn | null {
if (ts.isArrowFunction(expr) || ts.isFunctionExpression(expr)) {
return expr;
}
if (ts.isCallExpression(expr)) {
const first = expr.arguments[0];
- return first ? unwrapToFn(ts, first) : null;
+ return first ? peel(ts, first, resolve, seen) : null;
+ }
+ if (!resolve || !ts.isIdentifier(expr)) {
+ return null;
+ }
+ const declaration = resolve(expr);
+ // `const A = B, B = A` never runs, but it parses — and half-written code is
+ // exactly what this tool gets pointed at.
+ if (!declaration || seen.has(declaration)) {
+ return null;
+ }
+ seen.add(declaration);
+ return fnOfDecl(ts, declaration, resolve, seen);
+}
+
+function fnOfDecl(
+ ts: TypeScriptApi,
+ declaration: TS.Declaration,
+ resolve: DeclarationResolver | undefined,
+ seen: Set,
+): ComponentFn | null {
+ if (ts.isFunctionDeclaration(declaration)) {
+ return declaration;
+ }
+ if (ts.isVariableDeclaration(declaration) && declaration.initializer) {
+ return peel(ts, declaration.initializer, resolve, seen);
}
return null;
}
diff --git a/packages/prop-flow/src/classify.ts b/packages/prop-flow/src/classify.ts
index 3ebdb7e2..3ffeb824 100644
--- a/packages/prop-flow/src/classify.ts
+++ b/packages/prop-flow/src/classify.ts
@@ -68,6 +68,15 @@ export function createClassifier({ checker, components, ts }: ClassifierOptions)
// ── call site ─────────────────────────────────────────────────────────────
function classifyElement(el: TS.JsxOpeningLikeElement, propName: string): Classification {
+ // Nesting is the one value that is not written in the attributes at all, so
+ // no amount of reading them can find it. It also wins over everything that
+ // IS written there — `{y}` passes `y`, and so does
+ // `{y}` — which is why it comes first and short-circuits.
+ const nested = propName === 'children' ? nestedChildren(el) : null;
+ if (nested) {
+ return nested;
+ }
+
const { attr, spreadsAfter } = findAttr(ts, el, propName);
const candidates = spreadsAfter
.map((spread) => ({ carry: carryOf(spread, propName), spread }))
@@ -90,6 +99,47 @@ export function createClassifier({ checker, components, ts }: ClassifierOptions)
return classifySpread(winner.spread, propName);
}
+ /**
+ * What the element nests, as a value for `children` — or null when it nests
+ * nothing, which is when the attributes get their turn. A self-closing tag
+ * nests nothing by construction, and so does ``.
+ */
+ function nestedChildren(el: TS.JsxOpeningLikeElement): Classification | null {
+ if (!ts.isJsxOpeningElement(el) || !ts.isJsxElement(el.parent)) {
+ return null;
+ }
+ const children = el.parent.children.filter((child) => reachesChildren(child));
+ const [only] = children;
+ if (only === undefined) {
+ return null;
+ }
+ // A lone `{expr}` is the one nesting whose value can be read — and the only
+ // one that can be a pass-through: `{children}` forwards a prop
+ // rather than originating anything. Everything else — text, an element, or
+ // several children at once — is a value made right here, and an array of
+ // them is not a value any constancy claim can be made about.
+ if (children.length === 1 && ts.isJsxExpression(only) && only.expression) {
+ return classifyExpression(only.expression);
+ }
+ return { kind: 'real', note: 'nested children', value: null };
+ }
+
+ /**
+ * Whether a JSX child reaches `children` at all. Whitespace between tags and
+ * a lone `{/* comment *\/}` are both dropped before the element is built, so
+ * neither is a value — treating them as one would report `\n` as
+ * passing something.
+ */
+ function reachesChildren(child: TS.JsxChild): boolean {
+ if (ts.isJsxText(child)) {
+ return !child.containsOnlyTriviaWhiteSpaces;
+ }
+ if (ts.isJsxExpression(child)) {
+ return child.expression !== undefined;
+ }
+ return true;
+ }
+
/** What `{...x}` can contribute to `propName`, judged by its type alone. */
function carryOf(spread: TS.JsxSpreadAttribute, propName: string): Carry {
const type = checker.getTypeAtLocation(spread.expression);
diff --git a/packages/prop-flow/src/component.ts b/packages/prop-flow/src/component.ts
index 3b9b5fec..27103da6 100644
--- a/packages/prop-flow/src/component.ts
+++ b/packages/prop-flow/src/component.ts
@@ -1,5 +1,5 @@
import type * as TS from 'typescript';
-import { bindingNameOfFn, unwrapToFn } from './ast.js';
+import { bindingNameOfFn, fnOfDeclaration, unwrapToFn } from './ast.js';
import type { ComponentFn } from './ast.js';
import type { TypeScriptApi } from './typescript-api.js';
@@ -16,6 +16,12 @@ export interface ComponentFactoryOptions {
}
export interface ComponentFactory {
+ /**
+ * The component function a symbol ultimately names, wrappers peeled. The one
+ * thing two names for one component — `Card` and the `CardComponent` its
+ * `memo()` wraps — have in common, and so the identity to key on.
+ */
+ fnOfSymbol(sym: TS.Symbol): ComponentFn | null;
/** Resolve an `export { X }` specifier back to the function it names. */
fromExport(element: TS.ExportSpecifier): Component | null;
/** Rebuild a component descriptor from the function alone. */
@@ -24,6 +30,8 @@ export interface ComponentFactory {
fromNode(fn: ComponentFn, nameNode: TS.Identifier): Component | null;
/** Follow an import alias to the symbol it ultimately names. */
resolveAlias(sym: TS.Symbol): TS.Symbol;
+ /** Peel `memo(Inner)` and friends, following a name to what it declares. */
+ unwrap(expr: TS.Expression): ComponentFn | null;
}
/**
@@ -32,7 +40,15 @@ export interface ComponentFactory {
* cycle guard key on.
*/
export function createComponentFactory({ checker, ts }: ComponentFactoryOptions): ComponentFactory {
- return { fromExport, fromFn, fromNode, resolveAlias };
+ return { fnOfSymbol, fromExport, fromFn, fromNode, resolveAlias, unwrap };
+
+ function fnOfSymbol(sym: TS.Symbol): ComponentFn | null {
+ return fnOfDeclaration(ts, declarationOf(resolveAlias(sym)), resolveIdentifier);
+ }
+
+ function unwrap(expr: TS.Expression): ComponentFn | null {
+ return unwrapToFn(ts, expr, resolveIdentifier);
+ }
function fromExport(element: TS.ExportSpecifier): Component | null {
const exported = checker.getSymbolAtLocation(element.propertyName ?? element.name);
@@ -44,7 +60,7 @@ export function createComponentFactory({ checker, ts }: ComponentFactoryOptions)
return fromNode(declaration, declaration.name);
}
if (ts.isVariableDeclaration(declaration) && ts.isIdentifier(declaration.name) && declaration.initializer) {
- const fn = unwrapToFn(ts, declaration.initializer);
+ const fn = unwrap(declaration.initializer);
return fn ? fromNode(fn, declaration.name) : null;
}
return null;
@@ -67,4 +83,14 @@ export function createComponentFactory({ checker, ts }: ComponentFactoryOptions)
function resolveAlias(sym: TS.Symbol): TS.Symbol {
return (sym.getFlags() & ts.SymbolFlags.Alias) !== 0 ? checker.getAliasedSymbol(sym) : sym;
}
+
+ /** What a symbol declares — aliases are the caller's to resolve first. */
+ function declarationOf(sym: TS.Symbol): TS.Declaration | null {
+ return sym.valueDeclaration ?? sym.declarations?.[0] ?? null;
+ }
+
+ function resolveIdentifier(id: TS.Identifier): TS.Declaration | null {
+ const sym = checker.getSymbolAtLocation(id);
+ return sym ? declarationOf(resolveAlias(sym)) : null;
+ }
}
diff --git a/packages/prop-flow/src/report.test.ts b/packages/prop-flow/src/report.test.ts
index 7307fb63..2704ff91 100644
--- a/packages/prop-flow/src/report.test.ts
+++ b/packages/prop-flow/src/report.test.ts
@@ -137,6 +137,38 @@ describe('hint', () => {
])('%s (hasDefault=%s) advises %s', (verdict, hasDefault, expected) => {
expect(hint(makeRow({ hasDefault, verdict }))).toContain(expected);
});
+
+ it('names the pass-through behind a manual that was a caller-dead', () => {
+ // The counts still read "nobody passes it", so the line has to say why
+ // that is not the conclusion — and where the unseen caller could be.
+ const row = makeRow({
+ omit: 2,
+ real: 0,
+ sites: [
+ { kind: 'omit', loc: 'src/app.tsx:4:5' },
+ { kind: 'silent', loc: 'src/relay.tsx:9:7', note: 'contributed nothing', via: 'Relayed.note' },
+ ],
+ verdict: 'manual',
+ });
+
+ expect(hint(row)).toContain('Relayed.note contributed nothing');
+ expect(hint(row)).toContain('Check them before removing the prop');
+ // The generic manual advice would send the reader to sites that are not there.
+ expect(hint(row)).not.toContain('Check the MANUAL sites');
+ });
+
+ it('names each silent pass-through once', () => {
+ const row = makeRow({
+ sites: [
+ { kind: 'silent', loc: 'src/a.tsx:1:1', via: 'Relayed.note' },
+ { kind: 'silent', loc: 'src/b.tsx:2:2', via: 'Relayed.note' },
+ { kind: 'silent', loc: 'src/c.tsx:3:3', via: 'Other.note' },
+ ],
+ verdict: 'manual',
+ });
+
+ expect(hint(row)).toContain('Relayed.note, Other.note contributed nothing');
+ });
});
describe('constantHint', () => {
diff --git a/packages/prop-flow/src/report.ts b/packages/prop-flow/src/report.ts
index 67b77fa7..aecf0560 100644
--- a/packages/prop-flow/src/report.ts
+++ b/packages/prop-flow/src/report.ts
@@ -78,8 +78,13 @@ export function hint(row: PropReport): string {
return ' → no caller ever passes it; it is always `undefined` inside. Remove the prop and the code that reads it.\n';
case 'justified':
return ' → genuinely sometimes-absent. The `?` is correct.\n';
- case 'manual':
+ case 'manual': {
+ const silent = row.sites.filter((site) => site.kind === 'silent');
+ if (silent.length > 0) {
+ return silentHint(silent);
+ }
return ' → an unreadable spread or a contested override blocks a static verdict. Check the MANUAL sites by hand.\n';
+ }
case 'unused-component':
return ' → the component has no call sites in this Program. Verify the tsconfig spans its callers.\n';
// `required` and `cycle` have no advice of their own: what puts a required
@@ -89,6 +94,20 @@ export function hint(row: PropReport): string {
}
}
+/**
+ * What is left of a `caller-dead` that a silent pass-through pulled back. The
+ * counts still read "nobody passes it", so the line has to say why that is not
+ * the conclusion — and name the pass-through, which is the only place a caller
+ * this walk cannot see could be hiding.
+ */
+function silentHint(silent: readonly Site[]): string {
+ const traces = [...new Set(silent.map((site) => site.via ?? '?'))].join(', ');
+ return (
+ ` → every readable caller omits it, but ${traces} contributed nothing — its own call sites are\n` +
+ ' invisible here. Check them before removing the prop.\n'
+ );
+}
+
/**
* Constant does not mean wrong — `variant="danger"` on the two delete buttons
* is constant and correct. So the wording stops at "could be inlined".
diff --git a/packages/prop-flow/src/types.ts b/packages/prop-flow/src/types.ts
index 2a548692..bd8340b1 100644
--- a/packages/prop-flow/src/types.ts
+++ b/packages/prop-flow/src/types.ts
@@ -7,7 +7,10 @@
* unused-component the component itself has no call sites in the Program
* manual an unreadable spread, a dynamic value or a contested
* override blocks a static conclusion → listed for a
- * human to check
+ * human to check. Also where a `caller-dead` lands when
+ * a pass-through below it came back empty: the counts
+ * say "nobody passes it", but one of the sites they rest
+ * on said nothing at all
* cycle the pass-through graph looped back on itself; the
* repeat visit contributes no new information
* required the prop has no `?` to judge — listed only because it
@@ -16,7 +19,13 @@
export type Verdict =
'caller-dead' | 'cycle' | 'justified' | 'manual' | 'required' | 'unnecessary-optional' | 'unused-component';
-export type SiteKind = 'manual' | 'omit' | 'passthrough' | 'real';
+/**
+ * `silent` is a `passthrough` whose subtree moved no counter at all. It is kept
+ * apart because it is the one site that says nothing while looking like it
+ * said something: the row below it is empty, and an empty subtree is
+ * indistinguishable from a subtree that legitimately had nothing to add.
+ */
+export type SiteKind = 'manual' | 'omit' | 'passthrough' | 'real' | 'silent';
/** One JSX call site, classified. */
export interface Site {
@@ -25,9 +34,10 @@ export interface Site {
/** Why it was classified this way; a `passthrough` carries one only rarely. */
readonly note?: string;
/**
- * `Component.prop` the value was traced through. A `passthrough` always
- * carries one; so does a `real` site whose value is a default that fired one
- * level up, which is where the trace says *whose* default it was.
+ * `Component.prop` the value was traced through. A `passthrough` and a
+ * `silent` always carry one; so does a `real` site whose value is a default
+ * that fired one level up, which is where the trace says *whose* default it
+ * was.
*/
readonly via?: string;
}
diff --git a/packages/prop-flow/src/usage-index.test.ts b/packages/prop-flow/src/usage-index.test.ts
index 942eb2cb..fe21439c 100644
--- a/packages/prop-flow/src/usage-index.test.ts
+++ b/packages/prop-flow/src/usage-index.test.ts
@@ -85,7 +85,21 @@ describe('createUsageIndex', () => {
// `export { Aliased as Public }`, rendered as in app.tsx. Without
// alias resolution this would be a second entry nothing ever asks about,
// and Aliased would look like it had no call sites at all.
- expect(index.usagesOf(symbolOf('late.tsx', 'Aliased')).map(describeUsage)).toEqual(['Public@27']);
+ expect(index.usagesOf(symbolOf('late.tsx', 'Aliased')).map(describeUsage)).toEqual(['Public@28']);
+ });
+
+ it('files a usage under the function a wrapper call names, not under the binding', () => {
+ // `export const Chrome = memo(ChromeComponent)`: JSX renders `Chrome`,
+ // while a pass-through climbing out of the function body arrives at
+ // `ChromeComponent`. Both have to reach the same entry, or the climb ends
+ // on an empty usage list and the prop below it reads as caller-dead.
+ const wrapped = ['Chrome@38', 'Chrome@39'];
+ const binding = symbolOf('wrapped.tsx', 'Chrome');
+ const inner = symbolOf('wrapped.tsx', 'ChromeComponent');
+
+ expect(index.usagesOf(binding).map(describeUsage)).toEqual(wrapped);
+ expect(index.usagesOf(inner).map(describeUsage)).toEqual(wrapped);
+ expect(index.symbolId(binding)).toBe(index.symbolId(inner));
});
it('reports no usages for a component the Program never renders', () => {
diff --git a/packages/prop-flow/src/usage-index.ts b/packages/prop-flow/src/usage-index.ts
index e6559606..3c1aad4e 100644
--- a/packages/prop-flow/src/usage-index.ts
+++ b/packages/prop-flow/src/usage-index.ts
@@ -16,9 +16,14 @@ export interface UsageIndex {
*/
isCalled(sym: TS.Symbol): boolean;
/**
- * A stable number for `sym`. Symbols are objects, so a Map would key on them
- * directly — the id exists because it is printable, which the caches and the
- * cycle guard built on top of it both want.
+ * A stable number for the component `sym` names. Symbols are objects, so a
+ * Map would key on them directly — the id exists because it is printable,
+ * which the caches and the cycle guard built on top of it both want.
+ *
+ * Two names for ONE component share an id: `export const Card =
+ * memo(CardComponent)` is rendered as `Card` and climbed into as
+ * `CardComponent`, and a walk that filed those apart would find no call site
+ * on the way up — a live prop reported `caller-dead`.
*/
symbolId(sym: TS.Symbol): number;
/** Every JSX element rendering `sym`, in Program order. */
@@ -26,18 +31,24 @@ export interface UsageIndex {
}
/**
- * Every JSX usage in the Program, indexed in one walk: component symbol → call
- * sites, plus the symbols that are CALLED rather than rendered. Building this
+ * Every JSX usage in the Program, indexed in one walk: component → call sites,
+ * plus the components that are CALLED rather than rendered. Building this
* eagerly costs one traversal and saves one per prop analysed.
*
- * Import aliases are resolved on the way in, so `` and the `Aliased`
- * it was exported as land on the same entry.
+ * Both ways a component can wear a second name are resolved on the way in:
+ * `` and the `Aliased` it was exported as land on the same entry, and
+ * so do `` and the function its `memo()` wraps.
*/
export function createUsageIndex({ checker, components, program, ts }: UsageIndexOptions): UsageIndex {
- // Symbols are not primitives, so a WeakMap keyed by symbol is the identity
- // map; the id it hands out is what everything downstream keys on.
- const symbolIds = new WeakMap();
- let nextSymbolId = 1;
+ // Neither symbols nor functions are primitives, so a WeakMap keyed by the
+ // thing itself is the identity map; the id it hands out is what everything
+ // downstream keys on. One map for both kinds of key, so that one counter
+ // cannot hand the same number to a symbol and to a function.
+ const ids = new WeakMap();
+ let nextId = 1;
+ // What each symbol resolved to, memoised: `symbolId` runs on every JSX tag
+ // and every callee in the Program, and peeling a wrapper is not free.
+ const keys = new WeakMap();
const calledIds = new Set();
const usagesById = new Map();
@@ -60,11 +71,32 @@ export function createUsageIndex({ checker, components, program, ts }: UsageInde
}
function symbolId(sym: TS.Symbol): number {
- let id = symbolIds.get(sym);
+ return idOf(keyOf(sym));
+ }
+
+ /**
+ * What a symbol's usages are filed under: the component function it names,
+ * so a wrapper binding and the function inside it meet on one entry. A symbol
+ * naming no function — an intrinsic-like tag, a namespace member — keys on
+ * itself, which is the identity it had before.
+ */
+ function keyOf(sym: TS.Symbol): object {
+ const known = keys.get(sym);
+ if (known) {
+ return known;
+ }
+ const resolved = components.resolveAlias(sym);
+ const key: object = components.fnOfSymbol(resolved) ?? resolved;
+ keys.set(sym, key);
+ return key;
+ }
+
+ function idOf(key: object): number {
+ let id = ids.get(key);
if (id === undefined) {
- id = nextSymbolId;
- nextSymbolId += 1;
- symbolIds.set(sym, id);
+ id = nextId;
+ nextId += 1;
+ ids.set(key, id);
}
return id;
}
@@ -84,7 +116,7 @@ export function createUsageIndex({ checker, components, program, ts }: UsageInde
if (!sym) {
return;
}
- const id = symbolId(components.resolveAlias(sym));
+ const id = symbolId(sym);
const list = usagesById.get(id) ?? [];
list.push(node);
usagesById.set(id, list);
@@ -93,7 +125,7 @@ export function createUsageIndex({ checker, components, program, ts }: UsageInde
function recordCall(node: TS.CallExpression): void {
const sym = checker.getSymbolAtLocation(node.expression);
if (sym) {
- calledIds.add(symbolId(components.resolveAlias(sym)));
+ calledIds.add(symbolId(sym));
}
}
}