Merge monorepo into spotlightjs spotlight #1087
Merged
+1,512
−14,757
Conversation
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
Merges overlay, sidecar, and electron packages into a single spotlight package with improved architecture. ## Changes ### Package Structure - Consolidated three packages into `@spotlightjs/spotlight`: - `src/ui/` - UI components (formerly overlay) - `src/sidecar/` - Sidecar server functionality - `src/electron/` - Electron app code - `src/shared/` - Shared constants between UI and sidecar - Deleted `packages/overlay/`, `packages/sidecar/`, `packages/electron/` ### Build Configuration - Renamed `vite.overlay.config.ts` to `vite.ui.config.ts` - Updated output directory from `dist/overlay/` to `dist/ui/` - Merged all dependencies and scripts into single package.json - Updated tsconfig.json with path mappings for new structure ### Shared Code Architecture - Created `src/shared/constants.ts` for code shared between UI and sidecar - Prevents bundling sidecar code into UI library - Both UI and sidecar import from shared directory ### Import Updates - Updated ~400+ import paths across codebase - Fixed all references from old package structure - Updated all "overlay" terminology to "ui" ### Infrastructure Updates - Updated root package.json scripts (`dev:overlay` → `dev:ui`) - Updated turbo.json to remove overlay-specific tasks - Updated CI/CD pipeline in `.github/workflows/build.yml` - Updated `.cursorrules` documentation ### Testing - Build verified successful - CLI binary builds correctly - All output directories correctly named
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Fixes test failures by properly resolving sidecar path aliases in vitest. Uses regex-based alias matching to handle the .js extensions in imports.
Co-authored-by: burak.kaya <[email protected]>
Fixed several issues with the consolidated package: 1. **Sidecar compilation**: Created tsconfig.sidecar.json to compile sidecar TypeScript to JavaScript with NodeNext module resolution. The base tsconfig had emitDeclarationOnly:true which was preventing JS output. 2. **Binary bundling**: Updated build:sea script to explicitly specify bin/run.js as the entrypoint for fossilize, preventing it from bundling the UI library code (which caused "document is not defined" errors in Node). 3. **Asset path**: Fixed ENTRY_POINT_NAME from "src/index.html" to "index.html" to match the Vite manifest output. 4. **Binary name**: Added -o spotlight flag to fossilize to generate spotlight-linux-x64 binary name instead of run-linux-x64. 5. **Package exports**: Updated main/module/types to point to sidecar (dist/sidecar/main.js) as the primary export, and added "./ui" export for the UI library. This ensures users get the sidecar by default when importing the package. The spotlight binary now starts correctly and shows help output.
Added emptyOutDir: false to vite.config.ts to prevent Vite from cleaning the entire dist directory, which was removing the sidecar JavaScript files compiled by TypeScript. Build order: 1. tsc (UI types) 2. tsc sidecar (sidecar JS files to dist/sidecar/) 3. vite build (UI library to dist/) - now preserves dist/sidecar/ 4. vite build UI static assets (to dist/ui/) 5. fossilize SEA binary The spotlight binary now builds and runs correctly without issues.
Resolved merge conflicts from main branch while maintaining our consolidated monorepo structure: - Kept our consolidated directory structure (src/ui/, src/sidecar/, bin/) - Kept our build process (tsc for sidecar, vite for UI, fossilize for binary) - Updated imports to use our internal structure instead of @spotlightjs/* packages - Merged UI improvements (EventAttachment type, attachment handling) - Kept bin/run.js and bin/instrument.js (main has them in src/) - Removed eventTarget.ts (deleted in main, not used) Main branch changes incorporated: - Attachment support in telemetry - Documentation updates - CLI help improvements
Aligned with main branch changes that removed the embedded UI overlay model: - Removed sendEvent, onSevereEvent, trigger, on exports - Removed eventTarget module (deleted in main) - Removed WindowWithSpotlight type usage - Simplified electron-renderer to not use badge count events - Fixed unused Envelope import This matches the architectural change where Spotlight moved away from embedded UI to standalone UI and Electron app models. Binary and tests verified working.
Added proper path alias configuration to electron.vite.config.js: - main/preload: ~ points to src/sidecar (for sidecar imports) - renderer: ~ points to src/ui (for UI imports) Added required plugins to renderer config: - react plugin for JSX transformation - svgr plugin for SVG imports as React components Both electron dev and production builds now work correctly. The spotlight binary continues to work without issues.
- Updated version to 4.5.0 from main branch - Fixed binary startup: serve index.html with correct path "src/index.html" to match serveFilesHandler expectations The binary now starts successfully and serves the UI correctly.
Merged commits: - b3a654a: Improve SDK categorization implementation (#1110) - 43318aa: ci: Fix notarization env var swap (#1109) Key changes from main: - Enhanced SDK categorization using multiple signals (User-Agent, runtime tags, platform, server-specific signals) for better browser vs server detection - Fixed notarization environment variables in CI - Updated formatters to use improved categorization logic Conflict resolutions: - Kept our consolidated package structure (ui/, sidecar/, electron/) - Maintained separate build configs (vite.config.ts for UI library, vite.ui.config.ts for static assets, tsc for sidecar) - Preserved path aliases and electron app description - Removed TypeScript types from bin/run.js since it's a JS file - Fixed duplicate RAW_TYPES import in envelopesSlice.ts - Added core dump files to .gitignore All SDK categorization improvements have been merged into our consolidated structure.
The new SDK categorization code imports ~/routes/stream/userAgent.js which needs to be explicitly mapped in the main tsconfig.json. This path was already working in tsconfig.sidecar.json via the ~/* wildcard, but the main tsconfig needs explicit mappings for sidecar imports since it uses ~/* for UI files.
Co-authored-by: burak.kaya <[email protected]>
This restores changes from PR #1093 that were reverted during the monorepo consolidation: 1. Moved bin/run.js → src/run.ts (with TypeScript types) 2. Moved bin/instrument.js → src/instrument.ts (with TypeScript types) 3. Created vite.sidecar.config.ts to build run.ts and instrument.ts as SSR bundles 4. Updated package.json: - bin.spotlight now points to dist/run.js instead of bin/run.js - start script updated to use dist/run.js - build script now includes vite build --config vite.sidecar.config.ts - build:sea now uses dist/run.js as entrypoint - build script now also runs build:electron at the end - Removed bin/ from files array Benefits: - Version number is now baked into the build (shows v4.5.0 instead of 'unknown') - Consistent with main branch's structure - TypeScript type checking for CLI scripts The build now: 1. Compiles TypeScript declarations (tsc --skipLibCheck) 2. Compiles sidecar to JS (tsc --project tsconfig.sidecar.json + tsc-alias) 3. Builds UI library (vite build) 4. Builds UI static assets (vite build --config vite.ui.config.ts) 5. Builds CLI scripts (vite build --config vite.sidecar.config.ts) 6. Builds single executable (fossilize) 7. Builds Electron app (electron-vite build)
The E2E tests were hanging because: 1. The Electron app build output (out/ directory) wasn't included in CACHED_BUILD_PATHS, so it wasn't being cached by the build job 2. The test-e2e job wasn't restoring the build cache at all, so it had no dist/, dist-bin/, or out/ directories Changes: - Added packages/*/out to CACHED_BUILD_PATHS env var - Added "Restore build cache" step to test-e2e job before running tests This ensures the E2E tests have access to the pre-built Electron app from the build job.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Destructuring Reverses Operation Details
The destructuring assignment swaps op and description when splitting the function name. getFunctionNameFromFrame returns module@functionName, but the code assigns module to op and functionName to description, which is backwards. This causes spans created from profile frames to have incorrect operation names and descriptions.
packages/spotlight/src/ui/telemetry/data/profiles.ts#L128-L129
spotlight/packages/spotlight/src/ui/telemetry/data/profiles.ts
Lines 128 to 129 in 2b7400a
There was a problem hiding this comment.
Bug: Badge count broken for severe events.
The removal of the Spotlight.onSevereEvent callback breaks the badge count functionality in the Electron app. The main process still registers an IPC listener set-badge-count in handleBadgeCount, but nothing triggers it anymore since the renderer no longer calls window.electronAPI.setBadgeCount when severe events occur. This leaves the app badge count permanently at zero regardless of error count.
packages/spotlight/src/electron-renderer.ts#L21-L22
spotlight/packages/spotlight/src/electron-renderer.ts
Lines 21 to 22 in e4139ea
| newLogsByTraceId.set(logItem.trace_id, logSet); | ||
| set({ logsByTraceId: newLogsByTraceId }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Stale State Overwrites Logs in Loop
Multiple log items from the same event are lost due to state capture outside the loop. The code captures logsById and logsByTraceId once before the loop at line 43, then each iteration creates a new Map from these stale captures. When there are multiple processedLogs, each iteration overwrites the previous iteration's updates, causing all but the last log item to be lost from the store.
| * This is used when we receive an event with a trace_id but don't have a trace yet. | ||
| */ | ||
| export function initializeTrace(event: SentryEvent): Trace { | ||
| const traceCtx = event.contexts!.trace; |
There was a problem hiding this comment.
Bug: Contexts: Assertion Before Validation
The non-null assertion on event.contexts! assumes contexts exists, but this assumption is only validated after accessing it. The guard on line 11 checks if traceCtx exists, but by that point event.contexts!.trace has already been accessed with a non-null assertion, which can throw if contexts is undefined. The assertion happens before the validation.
BYK
deleted the
cursor/merge-monorepo-into-spotlightjs-spotlight-7503
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Merges all the packages under
spotlight, ending thesidecar,overlay, andelectronpackages.Fixes #605.