mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
fix/flow-dir-export-throw
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
98fe7a86fc |
test(profiler): type component-names tests with the module's exported types (#771)
Unbreaks the **Dead Code** gate on main, red since #663 merged. ## What happened #663 branched before the knip workflow existed (#572), so its own PR checks never ran the gate. Its `component-names.ts` exports three types with no importers — `StrippedName`, `ComponentAnnotation`, `ComponentNameResolution` — which pushed the knip second pass to **218** parked issues against the **215** ceiling. Every open PR now inherits the red check (e.g. #687, whose own commit adds no parked items). Local runs hid this: counted the way `knip.jsonc` prescribes — against a tree with **no build output** — the failure reproduces exactly. ## The fix Import the types where the tests naturally need them, which also removes a wart: the two `renderComponentNameMiss(r as never)` casts existed precisely because the narrowing `ComponentNameResolution` enables was never written. The new `expectMiss` helper narrows properly and fails loudly if the resolver ever stops returning a miss. No ceiling change: 47 + 156 + 12 = **215**, exactly at the limit. Note the budget now has zero headroom — the next PR that parks an export will trip it, which is the gate working as designed. ## Checks - clean-tree `npm run knip` exit 0 (both passes) - component-names tests 18/18, tool-server test typecheck clean, prettier clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
|
f136c024cc |
fix(profiler): accept the component names the analyze report actually prints (#663)
Fixes #632. ## The problem `react-profiler-analyze` strips `Forget()` / `Memo()` / `ForwardRef()` wrappers for readability, then the drill-down tools matched the raw DevTools name with `===`. The report names a component and the very next call refuses it. React Compiler is on by default in Expo SDK 54, so this hits essentially every new Expo app. ## Reproduced live Real session on an Expo SDK 54 app (`experiments.reactCompiler: true`, RN 0.81.5) — 821 fiber renders, 16 commits. The report printed `` `ParallaxScrollView` [React Compiler] ``: ``` profiler-cpu-query component_cpu ParallaxScrollView → "_Component `ParallaxScrollView` not found in commit data._" profiler-cpu-query component_cpu Forget(ParallaxScrollView) → "_No CPU samples found during `Forget(...)` commits._" ``` The two messages differ, and that is the proof: "not found in commit data" means the name never matched; the other is emitted only *after* a match. **Two things beyond the report:** 1. **`profiler-commit-query mode=by_component` has the same bug** — same refusal, full table under the raw name. 2. **It is not React-Compiler-specific.** `Memo()` and `ForwardRef()` are stripped too; the same report printed `` `InnerScreen` [forwardRef] ``. ## The workflow was a deadlock The AST index is keyed on bare source identifiers, so `react-profiler-component-source` accepts only the name the query tools reject: | name | query tools | component-source | |---|---|---| | `IconSymbol` | ✗ not found | ✓ file + line + source | | `Forget(IconSymbol)` | ✓ resolves | ✗ not found | There was **no single name an agent could carry** from analyze → drill-down → read source. Fixing only the query half would have left the issue's stated workflow broken. ## Approach Match a query against each recorded name's **display form**, rather than stripping both sides. Stripping both sides looks equivalent and is not: - it would quietly answer a query for `Forget(Foo)` with `Memo(Foo)` when only the latter exists — the agent asked about the compiler-wrapped fiber and silently got the memo one; - it isn't even reliable, because the display stripper stops after 4 wrappers, so a deeply wrapped name still *displays* with a wrapper and would never match. Comparing against the display form compares against exactly the string that was printed. **Exact matches always win and are never widened.** `StaticContainer` and `Memo(StaticContainer)` both appear in one ordinary 18-second session — I checked all 70 distinct names in the dump, and that is the one real collision. Asking for the bare name returns the bare fiber, and the output now names the sibling instead of silently showing one of two. **Ambiguity is refused, not merged.** Where a display name maps to several distinct fibers the tools list the exact strings to retry with. `Memo(X)` and `X` are often one component's two fibers whose durations nest, so a merged total describes no real component; and picking the largest silently picks the memo wrapper's inclusive time, which is the wrong fiber for self-cost analysis. The AST lookups get the reverse fallback: raw → stripped → expo-router file suffix removed (the report shows `TabTwoScreen(./(tabs)/explore.tsx)`; the source declares `TabTwoScreen`). Loosening is safe there in a way it is not for commit data — a wrong guess against the index simply misses and returns `found: false`. ## Verified against the live session | Case | Result | |---|---| | `component_cpu ParallaxScrollView` | resolves to `Forget(ParallaxScrollView)` — was refused | | `by_component ParallaxScrollView` | full table + `> Resolved … (React Compiler). Either name works here.` | | `by_component Forget(ParallaxScrollView)` | **byte-identical to before**, no note | | `by_component ParallaxScroll` (typo) | miss + `Forget(ParallaxScrollView)` as a re-queryable suggestion | | `component-source Forget(IconSymbol)` | `found: true`, `icon-symbol.tsx:28` — was `found: false` | | `component-source Forget(TabTwoScreen(./(tabs)/explore.tsx))` | `found: true`, `app/(tabs)/explore.tsx:12` | ## Visible report change (intentional) Fixing the analyze-side lookup means wrapped components stop losing their source location. In the live session exactly one row changes — every other row is a node_modules component with no user source: ``` before: | `IconSymbol` [React Compiler] | 6 | 0.1ms | … | — | after: | `IconSymbol` [React Compiler] | 6 | 0.1ms | … | `ui/icon-symbol.tsx:28` | ``` ## Deliberately out of scope - **`profiler-combined-report.ts:448`** correlates leaks to components by substring-matching raw names into native symbols, so wrapped components are structurally excluded. Same bug class — *affected, deferred*, not unaffected. - **A deeper namespace bug, filed separately:** `react-profiler-stop` joins fiber metadata by name across two different namespaces, so wrapped fibers lose `parentName`, `hookTypes` and `isCompilerOptimized` entirely. Measured on this session: **41 wrapped fibers, 0 with any of the three**, versus 181/272 bare fibers carrying `parentName`. That is why the report's header says `React Compiler: ✗` while the same document tags six components `[React Compiler]`. ## Checks - 3104 tests pass (298 files); 18 new, built on names taken verbatim from the real session - the `annotateComponentName` move is a pure refactor — verified output-identical before anything else changed - `03-tag.ts`'s `ANIMATED_PATTERN` deliberately left alone (it matches unanchored and covers `Motion`/`Transition`, which the stripper must not) - tool `description:` fields unchanged, so the SpiderShield average is untouched; skills gate 10.0; prettier, eslint, both typechecks clean --- > **#687 is stacked on this branch** — it conflicts with this one in `profiler-commit-query.ts`'s imports, so it was rebased on top. Merge this first. |