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 1 commit 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
|
📝 WalkthroughWalkthroughThis PR adds IDE availability detection to warn users when opening a code block without VS Code or Cursor installed. A new utility module detects each IDE via system ChangesIDE Detection and User Warning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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.
| let message = null; | ||
| if (!anyInstalled) { | ||
| message = 'No supported IDE found. Please install VS Code or Cursor to use "Open in Code" feature.'; | ||
| } |
There was a problem hiding this comment.
Localize the no-IDE warning message instead of hardcoding English text.
This string is user-facing and should come from next-intl (or a message key resolved by the caller), not be embedded here.
As per coding guidelines, apps/web/client/src/**/*.{ts,tsx} must “Avoid hardcoded user-facing text; use next-intl messages/hooks instead”.
🤖 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 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.
| // Check if a supported IDE is installed | ||
| const ideDetection = await detectSupportedIDE(); | ||
| if (!ideDetection.anyInstalled) { | ||
| toast.warning(ideDetection.message || 'No supported IDE found. Please install VS Code or Cursor.'); |
There was a problem hiding this comment.
Avoid hardcoded fallback toast text; route through i18n.
Line 23 still hardcodes a user-facing message. Use a translated message key (or localized formatter) for the fallback path as well.
As per coding guidelines, apps/web/client/src/**/*.{ts,tsx} must “Avoid hardcoded user-facing text; use next-intl messages/hooks instead”.
🤖 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 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.
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