fix(devtools-bundler-core): open source files outside cwd and end bad open-source requests - #541
AlemTuzlak wants to merge 1 commit into
Conversation
… open-source requests addSourceToJsx strips cwd from the module path. A file outside cwd (a monorepo package, or Vite run from another directory) keeps its absolute path, and the open-source handler then prefixed cwd again. The editor got paths like /repo/apps/web/repo/packages/pkg/src/panel.tsx. The handler now uses the absolute path when the cwd-relative file does not exist and the absolute one does. A missing or malformed `source` returned without ending the response, so the request stayed pending until the browser timed out. It now answers 400. Fixes #281 Fixes #176 Refs #451
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: TanStack/devtools/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe handler now preserves applicable source paths outside the current working directory and returns a completed HTTP 400 response when the ChangesSource request handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The changes are mergeable. A pre-existing click-to-code edge case remains when an external directory’s name begins with the working directory’s name. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The fix supports source files outside the working directory and completes invalid requests. The previous implementation already allowed paths outside that directory, so the change does not establish a new maximum file-access scope. Risk remains dependent on who can reach the development server and the controls surrounding it. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The new HTTP 400 response for missing or malformed Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 32e150a
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/angular-devtools
@tanstack/devtools
@tanstack/devtools-a11y
@tanstack/devtools-bundler-core
@tanstack/devtools-client
@tanstack/devtools-rspack
@tanstack/devtools-ui
@tanstack/devtools-utils
@tanstack/devtools-vite
@tanstack/devtools-webmcp
@tanstack/devtools-event-bus
@tanstack/devtools-event-client
@tanstack/preact-devtools
@tanstack/react-devtools
@tanstack/solid-devtools
@tanstack/svelte-devtools
@tanstack/vue-devtools
commit: |
"Go to Source" opens a wrong path when the source file is outside the current working directory, for example in a monorepo package or when Vite runs from another directory. This PR makes the open-source handler keep the absolute path of such files. It also answers
400for a badsource, so the request no longer stays pending.🎯 Changes
addSourceToJsxstripsprocess.cwd()from the module path. A file outside cwd keeps its absolute path, and the handler then added cwd again:/repo/apps/web/repo/packages/pkg/src/panel.tsx.sourcereturned withoutres.end(), so the request stayed pending until the browser timed out (the server part of Click-to-source sends empty source= and hangs because setDisabledAfterClick(true) runs before reading highlightState.dataSource #451). It now answers400.✅ Checklist
pnpm test:pr, or these tests do not apply to this pull request.🚀 Release Impact
Testing
Commands run
vitest runinpackages/devtools-bundler-core: 200 tests pass. The 3 new and updated tests failed before the fix.eslint,tsc, andprettier --checkon the changed files: pass.pnpm test:pr.Manual test
apps/weband render a component frompackages/pkg. Or use the StackBlitz repro in Click-to-code does not work when command run from different directory #281.packages/pkg/src/...at the right line./__tsd/open-source?source=in the browser. Before: pending forever. After:400.How this PR makes testing easy
Unit tests in
utils.test.tscover a file outside cwd (the repo rootpackage.json) and the400answers.Linked issues
Fixes #281
Fixes #176
Refs #451 (the client part of #451 was fixed in
@tanstack/devtools@0.12.3)Risk / rollback
Low. If both the cwd-relative path and the absolute path exist, the cwd-relative path wins, as before. To undo, revert this PR.
🤖 Generated with Claude Code
Summary by CodeRabbit