Open sketch source#635
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an “Open Source” action for the currently active sketch so users can open the sketch’s index.ts/index.js in the OS-default editor via an IPC call to the Electron main process.
Changes:
- Add a new IPC event (
FileEvents.OpenSketchSourceFile) for opening a sketch source file. - Add a renderer-side helper and wire it into the ActiveSketch popout menu (“Open Source”).
- Implement a main-process handler that resolves
index.ts/index.jsfor a sketch module and opens it viashell.openPath.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/desktop/src/shared/Events.ts | Adds a new FileEvents IPC channel for opening sketch source files. |
| apps/desktop/src/renderer/components/ActiveSketch/openSketchSourceFile.ts | Renderer utility that invokes the new IPC handler and reports errors to the user. |
| apps/desktop/src/renderer/components/ActiveSketch/ActiveSketch.tsx | Adds “Open Source” item to the ActiveSketch menu. |
| apps/desktop/src/main/index.ts | Registers an ipcMain.handle for the new file-open action. |
| apps/desktop/src/main/handlers/openSketchSourceFile.ts | Main-process implementation that locates and opens index.ts/js for a sketch module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { electronApp, optimizer } from '@electron-toolkit/utils' | ||
| import { REDUX_DEVTOOLS, installExtension } from '@tomjs/electron-devtools-installer' | ||
| import { ProjectData } from '@hedron-gl/app-store' | ||
| import { openSketchSourceFile } from './handlers/openSketchSourceFile' |
There was a problem hiding this comment.
Use the existing @main/handlers/... path alias here for consistency with the other handler imports in this file. Mixing relative and aliased imports makes refactors (like moving index.ts) harder and can lead to duplicate module resolution paths.
| import { openSketchSourceFile } from './handlers/openSketchSourceFile' | |
| import { openSketchSourceFile } from '@main/handlers/openSketchSourceFile' |
There was a problem hiding this comment.
ignore this one @cale-bradbury. Our eslint rules dictate this sort of thing. They disallow going back up the tree (e.g. ../../foo) but this way round is fine.
| const tsPath = path.join(sketchesDir, moduleId, 'index.ts') | ||
| const jsPath = path.join(sketchesDir, moduleId, 'index.js') | ||
| let filePath = tsPath | ||
| if (fs.existsSync(tsPath)) { | ||
| filePath = tsPath | ||
| } else if (fs.existsSync(jsPath)) { | ||
| filePath = jsPath | ||
| } else { | ||
| return { success: false, error: 'Source file not found' } |
There was a problem hiding this comment.
moduleId is used to build a filesystem path without validation. Because the preload exposes a full ipcRenderer API, a compromised renderer could pass values like ../... (or use symlinks) to make the main process open arbitrary files. Please validate moduleId (e.g., reject path separators / ..), and ensure the resolved target path stays within sketchesDir before calling shell.openPath.
| const result = await window.electronApi.ipcRenderer.invoke( | ||
| FileEvents.OpenSketchSourceFile, | ||
| sketchesDir, | ||
| moduleId, | ||
| ) | ||
| if (!result?.success) { | ||
| alert(result?.error || 'Source file not found for this sketch.') |
There was a problem hiding this comment.
ipcRenderer.invoke can reject (e.g., if the handler throws or IPC is unavailable). Right now that would surface as an unhandled promise rejection from a click handler. Wrap the invoke in a try/catch and surface a controlled error message (or return the error to the caller) so the UI can fail gracefully.
| const result = await window.electronApi.ipcRenderer.invoke( | |
| FileEvents.OpenSketchSourceFile, | |
| sketchesDir, | |
| moduleId, | |
| ) | |
| if (!result?.success) { | |
| alert(result?.error || 'Source file not found for this sketch.') | |
| try { | |
| const result = await window.electronApi.ipcRenderer.invoke( | |
| FileEvents.OpenSketchSourceFile, | |
| sketchesDir, | |
| moduleId, | |
| ) | |
| if (!result?.success) { | |
| alert(result?.error || 'Source file not found for this sketch.') | |
| } | |
| } catch (error: unknown) { | |
| const message = | |
| (error && typeof error === 'object' && 'message' in error | |
| ? String((error as { message?: string }).message || '') | |
| : '') || 'An unexpected error occurred.' | |
| alert(`Failed to open source file: ${message}`) |
| const tsPath = path.join(sketchesDir, moduleId, 'index.ts') | ||
| const jsPath = path.join(sketchesDir, moduleId, 'index.js') | ||
| let filePath = tsPath |
There was a problem hiding this comment.
instead of all this (regenerating the path), do we want to just cache the path on load @funwithtriangles? might be handy if we ever support sketch root files with different names
There was a problem hiding this comment.
let's discuss today, don't have my head in this one right now
# Conflicts: # apps/desktop/src/renderer/components/ActiveSketch/ActiveSketch.tsx
Adds a dropdown option to help you quickly open the source index.ts/js for a sketch in it's default program