fix(argent): declare + ship tree-sitter so react-profiler-component-source finds components#287
Draft
latekvo wants to merge 1 commit into
Draft
fix(argent): declare + ship tree-sitter so react-profiler-component-source finds components#287latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
…s components react-profiler-component-source returned found:false for every component in a standard TS/TSX project. The bundled tool-server require()s "tree-sitter" and "tree-sitter-typescript" (native addons) at runtime, but the published @swmansion/argent declared neither as a dependency, so the require threw, was swallowed by loadTreeSitter()'s try/catch, and the AST index was always empty. The grammar/match logic was already correct — the pinned pair parses export-default-function, plain-function, and arrow components. Declare both as dependencies and mark them external in the tool-server esbuild bundle (esbuild can't inline .node addons). Also stop misreporting "<name> not found in <root>" when the parser failed to load — surface a clear "tree-sitter parser could not be loaded" message (and the indexed-file count on a genuine miss), which would have made this packaging gap obvious. The bug is invisible in the dev monorepo (which has tree-sitter present) and only affects installs of the published package.
e7331a1 to
e203511
Compare
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.
Problem
react-profiler-component-sourcereturns{ found: false }for every component in a standard TypeScript/Expo project. Reproduced live against the published@swmansion/argent@0.9.0for three components that genuinely exist:PokemonExploreEntry—export default function PokemonExploreEntry(...)ExploreScreen—export default function ExploreScreen()StatBar— plainfunction StatBar(...)returning JSXEach lookup returned in 1–3 ms (too fast to have walked the tree), and the result message —
not found in <root>— actively misleads (it reads as "the component doesn't exist").Root cause
The bundled tool-server resolves the parser at runtime via
_require("tree-sitter")/_require("tree-sitter-typescript")(utils/react-profiler/pipeline/06-resolve/ast-index.ts). Both are native addons (ship a.nodebinary), so esbuild leaves them as runtime externals — but the published package declares neither as a dependency, so therequirethrows, is swallowed byloadTreeSitter()'stry/catch, andbuildAstIndexWithDiagnosticsreturns an empty index withtreeSitterAvailable: falsebefore reading a single file.astIndex.index.get(name)is then alwaysundefined→found: false.The grammar/match logic itself is correct. The bug is invisible in the dev monorepo (which has tree-sitter present) and only bites installs of the published package — which is why it shipped.
Fix
tree-sitter+tree-sitter-typescriptin@swmansion/argent'sdependenciesso they install alongside the package.externalin the tool-server esbuild bundle (esbuild can't inline.nodeaddons) — explicit and robust.treeSitterAvailableis false, return a clear "tree-sitter parser could not be loaded" message; on a real miss, include the indexed-file count. This would have surfaced the packaging gap immediately.Testing
found: false.Foo(export default function),Bar(plain function),Baz(arrow const) —hasError: false.test/react-profiler/ast-index.test.ts(there were no ast-index tests). It builds the index over a fixture.tsxand assertstreeSitterAvailable === trueplus the three components at the right lines.vitest run→ passes with tree-sitter present; thetreeSitterAvailableassertion fails loudly if the parser dependency goes missing again.