fix(react-grab): keep third-party component names in stack context#323
Open
aidenybai wants to merge 7 commits into
Open
fix(react-grab): keep third-party component names in stack context#323aidenybai wants to merge 7 commits into
aidenybai wants to merge 7 commits into
Conversation
When a user selects an element rendered by a third-party component
(e.g. SquareIcon from lucide-react), the selection label correctly
displays the component name via the React fiber walk, but the stack
context appended to the copied payload filtered out frames whose
fileName lives in node_modules or vendor bundles. The result was a
payload that jumped straight to the nearest user-source ancestor
(e.g. TooltipTrigger), leaving the agent with no idea what component
was actually selected.
formatStackContext now emits a name-only line ("in SquareIcon") for
frames that have a real-looking component name but no user-source
file. This mirrors React's own component-stack format and preserves
the most informative signal without leaking misleading library paths.
hasFormattableFrames is updated to recognize these frames as
formattable so the fallback fiber-name path is no longer used when
the owner stack carries the component identity.
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Recovers the npm package each `node_modules`-resolved frame came from and appends it to the stack context (`in Square (lucide-react)` and, for RSC trees, `in Image (next at Server)`) so the agent sees what library the grabbed component actually belongs to instead of a bare name. The new `parse-package-name` util handles direct paths, pnpm/Bun/Yarn-PnP nested layouts, Vite optimized deps (incl. `deps_temp` and scope-flattened `@scope_name.js`), Webpack/Turbopack/Next.js layer prefixes (`(rsc)`, `(app-pages-browser)`, etc.), Windows backslashes, multi-slash paths, and common CDN URL shapes (esm.sh / unpkg / jsdelivr / skypack), with an allowlist + path-boundary anchoring to avoid false positives on user paths that happen to contain `@`. Also coalesces consecutive frames from the same package into a single line so a deeply nested Radix/MUI/Chakra tree no longer evicts the user's own component frames from the tight `maxLines=3` budget. E2E fixture adds `@radix-ui/react-dialog` to cover scoped-package round-tripping through Vite's flat `@radix-ui_react-dialog.js?v=...` form. Co-authored-by: Cursor <cursoragent@cursor.com>
Centralize the line-push + previousLibraryPackage bookkeeping in a small emit() helper so individual branches can't forget to update tracking state, compute componentName once per iteration instead of three times, and extract the resolved-source line builder into formatResolvedSourceLine so the main loop body reads top-to-bottom (compute > coalesce > server > library > source) without inline string concatenation noise. Also polish parse-package-name: use splitPathSegments consistently in findVersionedPackageInPath, and document why un-versioned scoped CDN paths like /npm/@types/foo intentionally return null. Co-authored-by: Cursor <cursoragent@cursor.com>
Wire vite-plus test runner into packages/react-grab so its pure utils can be unit-tested without spinning up Playwright, and codify the bundler matrix as 40 cases in test/parse-package-name.test.ts: - plain node_modules (scoped + unscoped) - pnpm hoist (incl. peer-dep variant), Yarn PnP zips, Bun cache - Vite optimized deps: deps/, deps_temp/, deps_temp_<hash>/, scope flattening (@radix-ui_react-dialog -> @radix-ui/react-dialog), internal chunk-<hash>.js rejection - Webpack/webpack-internal/Turbopack URL shapes - Windows backslash paths, multi-slash defensive - hoist meta-dir rejection (.bin, .cache) - CDN URLs: esm.sh (incl. v135/, stable/), unpkg, jsdelivr (incl. /npm/ + scoped), skypack /pin/ - ambiguous (un-versioned scoped) and untrusted-host fallbacks Chain `pnpm test` to run the unit suite before Playwright; expose `pnpm test:unit` for the unit-only path. Co-authored-by: Cursor <cursoragent@cursor.com>
…heck The hardcoded `KNOWN_PACKAGE_CDN_HOSTS` set existed only to prevent the `<name>@<version>` CDN heuristic from misfiring on user paths that incidentally contain `@` (`me@work`, `foo@bar.com`, twitter `@handle`). The discriminator is actually staring at us: real npm versions in CDN URLs always start with a digit (or `v<digit>` for skypack's pinned URLs), while the false-positive cases never do. Replacing the host allow-list with a `^v?\d/` version-prefix check removes a magic list that had to be maintained as new CDNs appeared, and as a bonus correctly recognizes self-hosted internal CDN URLs that the allow-list previously rejected. file:// URLs are still routed to the node_modules matcher rather than treated as CDN URLs. Co-authored-by: Cursor <cursoragent@cursor.com>
Three small elegance wins in the package-name parser: 1. Fold `PACKAGE_VERSION_PREFIX_REGEX` and the three-step `splitNameAtVersion` helper into a single `NAME_AT_VERSION_REGEX` (`^(.+)@v?\d/`). The new `matchNameAtVersion` is a one-liner that captures the name only when an npm-shaped version follows, so `me@work` and `foo@bar.com` are still rejected by the same discriminator without the intermediate slice/test plumbing. 2. Replace the imperative `let lastMatchEnd; for (...)` accumulator in `matchAfterLastPattern` with `[...matchAll(pattern)].at(-1)` for declarative "last match if any" semantics. 3. Inline the one-call `stripFileExtension` helper into `readViteOptimizedDepPackage` and split the destructure for readability; replace the index-based loop in `findVersionedPackageInPath` with `segments.entries()` so the look-ahead reads as `segments[index + 1]` instead of two `let`s. Co-authored-by: Cursor <cursoragent@cursor.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
5 tasks
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
2 participants
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
When the user selects an element rendered by a third-party component (e.g.
SquareIconfromlucide-react), the selection label correctly shows the component name (SquareIcon), but the stack context appended to the copied payload drops it. The pasted output looks like:The component the user actually selected (
SquareIcon) is missing entirely. The agent receiving this payload has no way to know what was selected — it only sees the raw<svg>and a stack that starts at the nearest user-source ancestor.Root cause
formatStackContextinpackages/react-grab/src/core/context.tsfilters owner-stack frames throughisSourceFile()and only keeps frames whosefileNamelooks like user-authored source. Library components innode_modules(or vendor bundles) fail that check, so frames likeSquareIcon (at .../lucide-react/.../square.js)get dropped silently. The label UI doesn't have this problem because it walks the fiber tree directly viagetComponentDisplayName.Fix
In
formatStackContext, when a frame has a real-looking componentfunctionNamebut no user-sourcefileName, emit a name-only line (\n in SquareIcon) instead of skipping it. This mirrors React's own component-stack format and preserves the most informative bit (the component identity) without leaking misleading library file paths into the payload.hasFormattableFramesis updated correspondingly so the fallback fiber-name path isn't used when the owner stack already carries the component identity.Testing
element-context.spec.tsselects alucide-react<SquareIcon>rendered inside the e2e app and asserts the clipboard containsin SquareIconplus the user-source ancestor (LibraryIconSection).LibraryIconSectionwas added toapps/e2e-app/src/App.tsxandlucide-reactwas added as a dependency to surface the third-party scenario in tests.pnpm typecheck,pnpm lint, andpnpm formatare clean.Summary by cubic
Fixes
react-grabso the copied stack context keeps third‑party component names and tags them with the originating package (and server when applicable). Collapses consecutive library frames to preserve user frames; simplifies package parsing with a single version regex and better CDN detection.Bug Fixes
formatStackContext, emit a name‑only line when a frame has a valid component name but no user‑source file; updatehasFormattableFramesto treat these as valid.lucide-reacticon and asserts the clipboard includes "in Square (lucide-react)" and the user‑source ancestor.New Features
maxLines; include server frames as "(pkg at Server)".parse-package-nameto recover package names acrossnode_modules, Vite optimized deps (incl.deps_temp*and flattened scopes), Webpack/Turbopack/Next URL shapes, Windows paths, and CDN URLs; switch to a version‑prefix check (no host allow‑list), treatfile://as filesystem paths, and collapse name/version detection into one regex with a cleaner last‑match search. Add a scoped@radix-ui/react-dialogfixture and e2e to verify scope round‑trips.emithelper and extracted resolved‑source builder); add unit tests forparse-package-nameand wirevp testto run before Playwright (test,test:unit); add Vitest config.Written for commit 54dabef. Summary will update on new commits.