fix: detect VS Code or Cursor installation for "Open in Code" feature…#3110
fix: detect VS Code or Cursor installation for "Open in Code" feature…#3110guangyang1206 wants to merge 2 commits into
Conversation
… (Issue onlook-dev#318) - Add IDE detection utility (ide-detection.ts) - Check if VS Code or Cursor is installed when "Open in Code" is clicked - Show toast warning if neither IDE is found - Closes onlook-dev#318
|
@guangyang1206 is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📝 WalkthroughWalkthroughAdds IDE availability detection (VS Code or Cursor) and registers translations; IdeManager now checks availability before opening code blocks and shows a localized toast warning if neither IDE is found while continuing to open the code view. ChangesIDE Detection and User Warning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/client/src/components/store/editor/ide/index.ts (1)
4-4: ⚡ Quick winUse a src path alias for IDE detection import.
Switch this relative import to
@/*or~/*alias to match repo import conventions.Proposed fix
-import { detectSupportedIDE } from './ide-detection'; +import { detectSupportedIDE } from "`@/components/store/editor/ide/ide-detection`";As per coding guidelines,
apps/web/client/src/**/*.{ts,tsx}should “Use path aliases@/* and ~/* for imports that map to apps/web/client/src/*”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/client/src/components/store/editor/ide/index.ts` at line 4, Replace the relative import for detectSupportedIDE with the project path alias: update the import statement in index.ts that currently reads import { detectSupportedIDE } from './ide-detection' to use the `@/` alias (e.g. import { detectSupportedIDE } from '`@/components/store/editor/ide/ide-detection`') so it follows the repo convention for files under apps/web/client/src; ensure the symbol detectSupportedIDE is unchanged and the new aliased path resolves via the existing tsconfig/webpack alias setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/client/src/components/store/editor/ide/ide-detection.ts`:
- Around line 59-62: The hardcoded user-facing string assigned to message when
!anyInstalled must be replaced with a localized message; update the code in
ide-detection.ts to obtain the text from next-intl instead of embedding English.
Either call useTranslations (e.g. const t = useTranslations('ide'); then set
message = t('noSupportedIDE') ) or change the surrounding API to accept a
resolved message key/prop (e.g. pass noIdeMessage into the function and assign
message = noIdeMessage) and remove the literal string; reference the message
variable and anyInstalled flag when applying the change.
- Around line 23-24: The platform-specific check uses `which ${command}` which
fails on Windows; update the detection logic around the dynamic import of
`execSync` to pick the right tool based on platform (use `where ${command}` when
process.platform === 'win32`, and `which ${command}` otherwise), call execSync
with the same { stdio: 'ignore' } and catch errors to return false, and keep
referencing the existing `execSync` import and `command` variable so behavior is
unchanged on Unix but works on Windows too.
In `@apps/web/client/src/components/store/editor/ide/index.ts`:
- Line 23: Replace the hardcoded fallback string in the toast call by routing it
through the app i18n utilities: when calling toast.warning(...) use the
next-intl message lookup instead of the literal text so the fallback becomes
something like t('ide.noSupportedIDE') (or the project-specific formatter hook)
and keep ideDetection.message || t('ide.noSupportedIDE') as the argument; update
the component where toast.warning is invoked (the line referencing toast.warning
and ideDetection.message in this file) to import/use the appropriate next-intl
hook (e.g., useTranslations or formatMessage) and add the new translation key to
the locale messages.
---
Nitpick comments:
In `@apps/web/client/src/components/store/editor/ide/index.ts`:
- Line 4: Replace the relative import for detectSupportedIDE with the project
path alias: update the import statement in index.ts that currently reads import
{ detectSupportedIDE } from './ide-detection' to use the `@/` alias (e.g. import {
detectSupportedIDE } from '`@/components/store/editor/ide/ide-detection`') so it
follows the repo convention for files under apps/web/client/src; ensure the
symbol detectSupportedIDE is unchanged and the new aliased path resolves via the
existing tsconfig/webpack alias setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e60589e-4ea0-437b-8b0d-346a80c98451
📒 Files selected for processing (2)
apps/web/client/src/components/store/editor/ide/ide-detection.tsapps/web/client/src/components/store/editor/ide/index.ts
| const { execSync } = await import('child_process'); | ||
| execSync(`which ${command}`, { stdio: 'ignore' }); |
There was a problem hiding this comment.
Make command detection cross-platform (Windows currently false-negatives).
Line 24 uses which, which is Unix-specific and will fail in typical Windows shells even when VS Code/Cursor is installed.
Proposed fix
- const { execSync } = await import('child_process');
- execSync(`which ${command}`, { stdio: 'ignore' });
+ const { execFileSync } = await import('child_process');
+ const lookupCmd = process.platform === 'win32' ? 'where' : 'which';
+ execFileSync(lookupCmd, [command], { stdio: 'ignore' });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { execSync } = await import('child_process'); | |
| execSync(`which ${command}`, { stdio: 'ignore' }); | |
| const { execFileSync } = await import('child_process'); | |
| const lookupCmd = process.platform === 'win32' ? 'where' : 'which'; | |
| execFileSync(lookupCmd, [command], { stdio: 'ignore' }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/client/src/components/store/editor/ide/ide-detection.ts` around
lines 23 - 24, The platform-specific check uses `which ${command}` which fails
on Windows; update the detection logic around the dynamic import of `execSync`
to pick the right tool based on platform (use `where ${command}` when
process.platform === 'win32`, and `which ${command}` otherwise), call execSync
with the same { stdio: 'ignore' } and catch errors to return false, and keep
referencing the existing `execSync` import and `command` variable so behavior is
unchanged on Unix but works on Windows too.
- Replace hardcoded toast string in IdeManager.openCodeBlock() with i18n key lookup - Add setIdeTranslation()/getIdeTranslation() so React layer can inject the t() function - Add ide.noSupportedIDE key to en.json - Inject t() from useCodeNavigation hook via setIdeTranslation - Keep backward-compatible fallback when tFn is not yet injected
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/client/src/components/store/editor/ide/index.ts (1)
9-15:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove the redundant re-export of
setIdeTranslation/getIdeTranslationto avoid duplicate named exports.These functions are already exported in their
export function ...declarations (lines 9-15) and are re-exported again at line 93 (export { setIdeTranslation, getIdeTranslation };), which can trigger a TypeScript duplicate-export error.Proposed fix
-export { setIdeTranslation, getIdeTranslation };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/client/src/components/store/editor/ide/index.ts` around lines 9 - 15, The file currently defines and exports setIdeTranslation and getIdeTranslation via their function declarations and then re-exports them again, causing a duplicate named export error; remove the redundant re-export statement that lists setIdeTranslation and getIdeTranslation (the export { setIdeTranslation, getIdeTranslation }; line) so the functions are only exported once, leaving the original export function declarations unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/client/src/app/project/`[id]/_components/left-panel/code-panel/code-tab/hooks/use-code-navigation.ts:
- Around line 29-32: The hook useCodeNavigation is not declared async but uses
await import('`@/components/store/editor/ide`') to call setIdeTranslation, which
breaks parsing; move the dynamic import out of the synchronous hook body—e.g.,
inside a useEffect within useCodeNavigation (or replace with void
import(...).then(module => module.setIdeTranslation(t)))—so that
setIdeTranslation is invoked after the component mounts and the hook remains
non-async; reference the dynamic import path '`@/components/store/editor/ide`' and
the setIdeTranslation symbol when making the change.
---
Duplicate comments:
In `@apps/web/client/src/components/store/editor/ide/index.ts`:
- Around line 9-15: The file currently defines and exports setIdeTranslation and
getIdeTranslation via their function declarations and then re-exports them
again, causing a duplicate named export error; remove the redundant re-export
statement that lists setIdeTranslation and getIdeTranslation (the export {
setIdeTranslation, getIdeTranslation }; line) so the functions are only exported
once, leaving the original export function declarations unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 128c1fde-29dd-4b14-9221-1dc3ffdca49d
📒 Files selected for processing (4)
apps/web/client/messages/en.jsonapps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/hooks/use-code-navigation.tsapps/web/client/src/components/store/editor/ide/ide-detection.tsapps/web/client/src/components/store/editor/ide/index.ts
| // Inject translation function into IdeManager | ||
| const { setIdeTranslation } = await import('@/components/store/editor/ide'); | ||
| setIdeTranslation(t); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the non-async await usage is present in this hook.
rg -n -C2 'export function useCodeNavigation|await import' "apps/web/client/src/app/project/[id]/_components/left-panel/code-panel/code-tab/hooks/use-code-navigation.ts"Repository: onlook-dev/onlook
Length of output: 372
Critical: remove await import() from non-async useCodeNavigation
useCodeNavigation is not declared async (line 21) but contains await import('@/components/store/editor/ide') (lines 29-31), which is a parse-time blocker. Move the dynamic import into useEffect (or use a void import(...).then(...) pattern) so the hook body has no await.
🧰 Tools
🪛 Biome (2.4.15)
[error] 30-30: await is only allowed within async functions and at the top levels of modules.
(parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/web/client/src/app/project/`[id]/_components/left-panel/code-panel/code-tab/hooks/use-code-navigation.ts
around lines 29 - 32, The hook useCodeNavigation is not declared async but uses
await import('`@/components/store/editor/ide`') to call setIdeTranslation, which
breaks parsing; move the dynamic import out of the synchronous hook body—e.g.,
inside a useEffect within useCodeNavigation (or replace with void
import(...).then(module => module.setIdeTranslation(t)))—so that
setIdeTranslation is invoked after the component mounts and the hook remains
non-async; reference the dynamic import path '`@/components/store/editor/ide`' and
the setIdeTranslation symbol when making the change.
1 participant
… (Issue #318)
Description
This PR implements IDE detection for the "Open in Code" feature. Currently, when users click "Open in Code" without having VS Code or Cursor installed, nothing happens and there is no feedback to the user.
This PR adds:
ide-detection.tsutility that checks if VS Code (code) or Cursor (cursor) is available in the system PATHIdeManager.openCodeBlock()to detect IDE availabilityRelated Issues
Closes #318
Type of Change
Testing
Screenshots
N/A (toast warning message)
Additional Notes
execSync('which <command>')and is only run in Node.js environment (not browser)true) to avoid false warningsPR Link: main...guangyang1206:onlook:fix/detect-ide-for-code-view-318
Summary by CodeRabbit