Repository navigation
fix(react): a name a component destructures, or a function around it binds, renders no JSX child - #2676
Merged
Conversation
…binds, renders no JSX child #2442 made a tag the parent binds itself, as a `const` or a parameter, render only a component the parent declares inside itself. Destructuring was left out, as name-matcher leaves it out for a bare call (`const { t } = useI18n()` is that function more often than not), so a destructured tag still linked any same-named component in the repository. For a tag that is the wrong default: what a component destructures is a component its props or a library hand in, which is no node here. - refine's layouts, `({ children, Header, Footer, Sider, OffLayoutArea }) =>`, rendered other examples' `Header`, `Footer`, `Sider` and `OffLayoutArea`; `const { Title } = Typography` (antd) and `const { Link } = useRouterContext()` rendered other examples' `Title` and `Link`. - excalidraw's example app, `const { Sidebar, Footer, WelcomeScreen, … } = excalidrawLib`, rendered its own `ExampleSidebar.tsx` for the library's `Sidebar`, and `Footer.tsx` though the library exports `FooterCenter` as `Footer`. Its other tags landed on the library's components by luck. A tag now checks the parent's code through its first tag for a destructuring pattern that binds the name (`destructures`, read off the code-only view): a declaration's or a parameter's, nested patterns, rest elements, rename targets and array patterns included. A rename's key, a default value and a type literal after a pattern bind nothing, nor does an object argument or a control-flow head, and a declaration of `require(…)` or `import(…)` binds the module's own export, which links as before. The same check, and #2442's `const`/parameter check, now also reads the functions around the parent: excalidraw's `renderExcalidraw` and `renderTopRightUI` render what `ExampleApp` destructures. The function that binds the name is the one whose own components a tag may still render (`declaredInside`), so a helper rendering a component its enclosing function declares (binner's `recursiveTreeItem` → `StyledTreeItem`) keeps its edge. Fresh before/after indexes on 31 repos (main a50104c): 24 jsx-render edges removed (excalidraw 14, refine-examples 10), every one graded all-local by the TypeScript checker (each tag a destructured local); 15 targets were wrong, 9 right by luck. One of them, ExampleApp → ExampleSidebar's `Sidebar`, comes back with `via` ExampleSidebar, the default import that renders it, which the wrong `<Sidebar>` edge used to hide. No other edge, node, ref or file changes; 29 repos are byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decision: a destructured JSX name links nothing
#2442 made a tag the component binds itself, a
constor a parameter, render only a component the parent declares inside itself. It left destructuring out on purpose, matching name-matcher's rule for calls:const { t } = useI18n()is usually that very function. A follow-up asked whether JSX should follow the call rule or the local rule. The data on current main says JSX should follow the local rule.A TypeScript-checker survey of every jsx-render edge on 31 repos (main a50104c) found 24 edges where every tag of the name is a destructured local. 15 of them link a wrong component, and 9 link the right one by luck:
examples/with-script-in-browserconst { Sidebar, Footer, WelcomeScreen, TTDDialog, … } = excalidrawLib({ children, Header, Footer, Sider, OffLayoutArea }) =>, a storeFooter's({ Icon })const { Title } = Typography(antd),const { Link } = useRouterContext()Sidebarreaches the example's ownsidebar/ExampleSidebar.tsx, not the library'sSidebar.Footerreachescomponents/footer/Footer.tsx, but the library exportsfooter/FooterCenterasFooter.TTDDialog,TTDDialogTrigger,WelcomeScreen,LiveCollaborationTrigger,Button) are right only because the example app lives inside the library's own repository. In an app that consumes a library, the same destructuring can only reach a stranger.What a component destructures is a component its props or a library hand it, and that component is no node in the graph. So silence beats a guess here, which is the reverse of the call case. Calls are unchanged:
jsCodeBindsNameand every name-matcher path are untouched.6 of the 14 excalidraw edges come from
renderExcalidrawandrenderTopRightUI, functions nested insideExampleAppthat render whatExampleAppdestructures. A check that reads only the parent's own lines would treat the same binding two ways. So the binding check reads the functions around the parent too, for #2442'sconst/parameter rule as well as for destructuring.What changes
In
reactJsxChildEdges(callback-synthesizer.ts):destructures(src, name, end)reads the parent's code-only view (jsxCode, where string contents are blank) through the tag's first line. It looks for a destructuring pattern that binds the name:const/let/var, includingfor (const { X } of …));if/while/for/switch/withprecedes it;{ icon: Icon }) and array patterns (const [Panel] = useState()).{ Footer: Custom });{ Header = DefaultHeader });}: { footer: Footer },props: { footer: Footer });render({ Footer }));require(…)orawait import(…), which binds the module's own export and links as before.const/parameter/destructuring check. The function that binds the name is the one passed todeclaredInside. So binner'srecursiveTreeItemstill renders theStyledTreeItemits enclosing component declares. Those 9 edges, one per plain local bound in an enclosing function, are unchanged in the A/B.spanTags, so an enclosing function's span is read on demand and cached like a parent's.Validation
Fresh before/after indexes on the 31 survey repos, compared with
scripts/dump-graph.mjs. Every changed jsx-render edge was graded by the TypeScript-checker oracle, which binds each tag occurrence withts.createProgramand lexical scope.On main a50104c:
all-local(8 bound in the parent, 6 inExampleApparoundrenderExcalidraw/renderTopRightUI). The +1 is one of the removed edges coming back:ExampleApp → SidebarinExampleSidebar.tsx, now withvia: ExampleSidebar. That is the default import<ExampleSidebar>that really renders it. Before, the wrong<Sidebar>edge took that slot in the dedup.all-local(7 destructured props, 3 destructured locals).I merged main twice more and re-ran the A/B both times:
ab6ec218: fix(js,ts): a function value imported from the project links only to what its import binds #2673, function values imported from the project, in name-matcher;bb8a0f73: fix(js): a JavaScript file's import of a TypeScript module links that module #2674, a JavaScript file's import of a TypeScript module, and fix(c,cpp): a name in a C or C++ test links to what the test's own includes declare #2675, C/C++ test includes.Both times, every repo's removed and added jsx-render edges matched the first run edge for edge, and the same 29 repos were byte-identical. Each pair of arms differed only in
callback-synthesizer.js, so the diff is this change and nothing else.Tests
__tests__/jsx-child-disambiguation.test.tsreplaces the test that pinned the old behaviour (destructured names linked by name) with three tests:renderSidebar; refine's layout props; a rename target; a pattern nested in a parameter's; an array pattern.constin the enclosing function renders nothing; a component the enclosing function declares still renders (the binner shape); a callback's destructuredCelldoesn't hide theCellthe table declares.,-separated, after a pattern and after a plain parameter), an object argument, and a destructuredrequire()andawait import().The new "renders nothing" tests fail on main. Mutations each break the assertions they guard:
declaredInsideagainst the parent instead of the binding functionrequire/import()exception offFull suite on the merged tree (main
bb8a0f73+ this branch), Windows 11 x64, Node 22, kernel built,CODEGRAPH_KERNEL_EXPECT=1,--maxWorkers=4 --minWorkers=1: 618 files passed, 6 skipped; 7,499 tests passed, 80 skipped, 0 failed.Cost
The
jsxEdgespass was timed in-process with an instrumented build, adding up only the new paths: the loop over functions around the parent, and everydestructuredBy.destructuredByThat is at most about 4 ms per repo, 0.1–0.3% of the pass. Interleaved median runs of the whole pass (5 rounds per arm) swung by ±15% on this shared machine, with binner "faster" after, so they can't resolve a change this small. No regex is compiled per name; the scan is
indexOfplus bracket walks capped at 10,000 characters.Overlap: #2611 (open, @inth3shadows)
#2611 attributes JSX outside every function to the top-level value or the file. It edits the same post-check line:
It also adds its own
SpanTags = { tags, bare, src }withsrcset only for owning parents. Whichever lands second can resolve it this way:SpanTags(withdestructured). Setsrcto the owned text for an owning parent, as fix(react): JSX at module scope, in a top-level const or in a test callback renders from where its calls are attributed (#2587) #2611 does, and to the span's code otherwise.ownLinesBind(name, names.src) || destructuredBy(names, name, …). That also lifts fix(react): JSX at module scope, in a top-level const or in a test callback renders from where its calls are attributed (#2587) #2611's "module-level destructuring links by name" known limit for tags it owns.scopeBindsas here.Not in this PR
const { Title, Text } = Typography;(antd), still links by name. The survey finds 177 such edges, all in refine-examples. 94 land on one example'sTextand 83 on another'sTitle, and every one is wrong. That is a different scope (the file's), which overlaps fix(react): JSX at module scope, in a top-level const or in a test callback renders from where its calls are attributed (#2587) #2611's area, so it goes in a follow-up PR.🤖 Generated with Claude Code