Custom Dashboards Versioning fix#1418
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughViewport-based height change in the dashboard page and version-stamping of generated/editor code with packageJson.version; sandbox iframe dependency loader now computes an ESM fallback, tries primary then fallback imports, reports structured dependency errors to the host, and halts initialization on failure. ChangesViewport-Based Height Calculation
Sandbox Dependency Error Handling & Resilience
Sequence DiagramsequenceDiagram
participant Host as Dashboard Host
participant Sandbox as Iframe Sandbox
participant Deps as Dependency Loader
Host->>Sandbox: Generate iframe HTML with esmVersion & esmFallbackVersion
Sandbox->>Deps: import Stack SDK (`@stackframe/js`@esmVersion)
alt Primary succeeds
Deps-->>Sandbox: SDK loaded
Sandbox->>Sandbox: set window.DashboardUI / window.Stack* and initialize
Sandbox-->>Host: deps-ready
else Primary fails
Deps-->>Sandbox: import error
Sandbox->>Deps: import Stack SDK (`@stackframe/js`@esmFallbackVersion)
alt Fallback succeeds
Deps-->>Sandbox: SDK loaded (fallback)
Sandbox->>Sandbox: set globals and initialize
Sandbox-->>Host: deps-ready
else Fallback fails
Deps-->>Sandbox: import error
Sandbox->>Sandbox: set window.__depsError (structured)
Sandbox-->>Host: dashboard-sandbox-dependency-error (payload with message and optional stack)
Host->>Host: captureError('dashboard-sandbox-dependency-error', Error)
Host->>Host: halt initialization
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Greptile SummaryThis PR introduces a version-stamping mechanism for custom dashboards and a versioned ESM fallback loader to prevent dashboard breakage when a newly published package version is not yet available on esm.sh.
Confidence Score: 4/5Safe to merge; the production load path is guarded correctly and the stamping logic is sound. The dev-mode gap is a cosmetic inconsistency that only affects local development. All three code paths that produce dashboard source correctly stamp and replace any existing stamp. The production ESM fallback logic is well-structured with explicit error state and guarded globals. The only issue found is in the dev-mode branch: after both SDK imports fail, execution continues past The dev-mode script block in Important Files Changed
Sequence DiagramsequenceDiagram
participant Parent as DashboardSandboxHost (React)
participant Sandbox as Sandbox iframe
participant ESM as esm.sh CDN
Parent->>Sandbox: srcdoc (HTML with stamped esmVersion)
Sandbox->>ESM: "import @stackframe/js@esmVersion"
alt primary succeeds
ESM-->>Sandbox: module loaded
Sandbox->>Sandbox: "set globals, __depsReady=true, fire deps-ready"
else primary fails
ESM-->>Sandbox: error
Sandbox->>Parent: postMessage dashboard-sandbox-dependency-error (Sentry)
Sandbox->>ESM: "import @stackframe/js@esmFallbackVersion"
alt fallback succeeds
ESM-->>Sandbox: module loaded
Sandbox->>Sandbox: "set globals, __depsReady=true, fire deps-ready"
else fallback fails
ESM-->>Sandbox: error
Sandbox->>Parent: postMessage dashboard-sandbox-dependency-error (Sentry)
Sandbox->>Sandbox: __depsError set, fire deps-ready
Sandbox->>Parent: postMessage stack-ai-dashboard-error (user-facing)
end
end
Sandbox->>Parent: postMessage stack-ai-dashboard-ready
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx:98-112
**Dev mode: no early exit after `failDependencyLoad`, IIFE still appended**
When both SDK imports fail in the dev-mode branch, `failDependencyLoad` fires `deps-ready` and sets `__depsError`, but execution continues unguarded. `window.generateUuid` is still assigned, a `<script>` tag for the IIFE bundle is still created and appended, and — if the bundle happens to exist — `script.onload` fires a second `deps-ready` event without checking `__depsError` first (unlike the production path which is guarded by `if (!window.__depsError)`). The second event is harmless because `{ once: true }` already consumed the first, but the inconsistency can cause confusing Sentry noise and wastes a network round-trip. Consider adding an early-return or an `if (!window.__depsError)` gate around `window.generateUuid` and the IIFE script creation, matching the production guard that already does this correctly.
Reviews (3): Last reviewed commit: "Merge branch 'dev' into custom-dashboard..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR updates the custom dashboard sandbox to better handle dependency version mismatches by adding fallback-loading behavior and improved error reporting from the iframe back to the parent dashboard UI.
Changes:
- Derives an ESM fallback version from the current app version and retries loading
@stackframe/*dependencies when the current version isn’t available. - Adds structured dependency-load error reporting (
dashboard-sandbox-dependency-error) and propagates dependency failures throughwaitForDeps(). - Adjusts the custom dashboard detail page container sizing via viewport-based height calculations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx | Adds version fallback logic for ESM imports and introduces dependency-load error reporting/handling between iframe and parent. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx | Changes the full-bleed dashboard container height to a 100vh - <offset> calculation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx (2)
30-163: ⚡ Quick winDependency-loader helpers are duplicated verbatim between the dev and prod branches.
formatDependencyError,reportDependencyError, andfailDependencyLoadare byte-for-byte identical at lines 34-54 (dev) and 103-123 (prod). The only intentional divergence is the user-facing message swap in prod'sfailDependencyLoad(line 118 usesCUSTOM_DASHBOARD_LOAD_ERROR_MESSAGE). Two parallel copies of error-reporting logic across two long inline-string scripts is exactly the kind of place a future bug fix gets applied to one copy and not the other.Suggested refactor: hoist the shared helpers into a single small string returned by a
getDependencyHelpersScript(userFacingMessage)function, prepend it to both branches, and let dev pass through the raw message ((_, raw) => raw) and prod return the canned string. The two branches then differ only in the import URLs they care about.🤖 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/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx` around lines 30 - 163, The dev and prod branches in getDependencyScripts duplicate the same helper functions (formatDependencyError, reportDependencyError, failDependencyLoad); extract those shared helpers into a new function getDependencyHelpersScript(userFacingMessage) that returns the helper-script string, then prepend its output to both the dev and prod module strings in getDependencyScripts; have dev call getDependencyHelpersScript with a pass-through/raw message and prod call it with CUSTOM_DASHBOARD_LOAD_ERROR_MESSAGE so only the message differs while imports and load logic remain in their respective branches (update references to formatDependencyError, reportDependencyError, and failDependencyLoad inside both inline scripts to use the hoisted helpers).
802-807: ⚡ Quick win
dashboard-sandbox-dependency-erroris captured to observability but never surfaced toonRuntimeError.The new handler only calls
captureErrorand returns. The user still eventually sees the error becausewaitForDepsrethrows on__depsError, which propagates throughinitializeStackApp().catch(...)→stack-ai-dashboard-error→onRuntimeErrorRef.current?.()at lines 824-827. That works, but it has two side effects worth confirming are intentional:
- Double reporting in observability. The same root cause is captured twice — once here (
dashboard-sandbox-dependency-error) and once in the boot-failure path (which on its own surface re-enters viadashboard-error-boundaryif the ErrorBoundary catches it, or via thestack-ai-dashboard-errorflow which does not callcaptureErroritself). If you're tracking dependency-load reliability separately, this is great; if not, you'll have noisy duplicates with different signatures.- Latency before the user sees the chat-prefill. The composer prefill in
handleDashboardRuntimeErroronly fires oncestack-ai-dashboard-errorarrives, which requires the boot promise to reject. If you want the chat composer to populate the moment the dependency load fails (rather than afterinitializeStackAppfinishes failing), forward the dependency error throughonRuntimeErrorRef.current?.(...)here as well — the ref-based debounce in the parent already coalesces duplicates within 2 s.Suggested addition
if (type === "dashboard-sandbox-dependency-error") { const err = new Error(event.data.message ?? 'Unknown custom dashboard dependency error'); if (event.data.stack) err.stack = event.data.stack; captureError('dashboard-sandbox-dependency-error', err); + onRuntimeErrorRef.current?.({ + message: typeof event.data.message === "string" ? event.data.message : "Custom dashboard dependency failed to load", + stack: typeof event.data.stack === "string" ? event.data.stack : undefined, + }); return; }The parent's
lastErrorRef2-second window will dedupe this against the subsequentstack-ai-dashboard-errorfor the same root cause, so the composer won't be stomped twice.🤖 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/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx` around lines 802 - 807, The dependency-error handler currently only calls captureError('dashboard-sandbox-dependency-error', err) and returns, which delays and duplicates user-facing error handling; after creating and capturing the Error (the err built from event.data), also invoke onRuntimeErrorRef.current?.(err) so the runtime error is surfaced to the parent immediately (this will be deduped by the parent's lastErrorRef window against the later stack-ai-dashboard-error), keeping the same Error instance so waitForDeps / initializeStackApp boot-failure flow still sees the original error if it rethrows.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx (1)
394-394: ⚖️ Poor tradeoffTheme-dependent viewport math is used consistently but relies on fragile hardcoded values.
The
flex h-[calc(100vh-4.5rem)] dark:h-[calc(100vh-5.75rem)]pattern appears intentionally throughout the codebase (includingsidebar-layout.tsxline 738, which hash-[calc(100vh-3.5rem)]light mode anddark:h-[calc(100vh-6rem)]dark mode). The1.25remdark-mode delta accounts for the header'sdark:top-3 dark:mx-3 dark:mb-3 dark:mt-3spacing applied in dark mode only.While the pattern is intentional, it remains fragile: if the header spacing or navbar height changes, these calculations silently drift. The DESIGN-GUIDE does not address height management, and there are no CSS variables centralizing these values.
Prefer refactoring to:
- Define shared CSS variables for header/navbar height on
:rootand.dark(e.g.,--app-header-height-lightand--app-header-height-dark), then useh-[calc(100vh-var(--app-header-height-light))].- Or let the parent flex container own viewport math so this page does not need to know header dimensions.
Also consider
100dvhover100vhif this view may render on small viewports.🤖 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/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx at line 394, The height calculation on the div with data-full-bleed using the class 'flex h-[calc(100vh-4.5rem)] dark:h-[calc(100vh-5.75rem)]' is fragile; replace the hardcoded per-theme math by introducing shared CSS variables (e.g., --app-header-height-light and --app-header-height-dark) set on :root and .dark (mirroring the header spacing that currently uses dark:top-3 etc.), then change the classes to use h-[calc(100vh-var(--app-header-height-light))] and dark:h-[calc(100vh-var(--app-header-height-dark))] or, even better, move the viewport calc to a parent flex container so this page component (the div with data-full-bleed) doesn't need to know header dimensions; also consider switching to 100dvh in the calc for mobile browser consistency.
🤖 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/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx`:
- Around line 23-28: getEsmFallbackVersion currently produces invalid fallback
versions for inputs like "2.0.0", prereleases, missing patch or empty strings;
instead validate the input and fail loudly: import and use the repo's throwErr
and in getEsmFallbackVersion assert the version is non-empty, split into exactly
three dot-separated parts, and that major/minor/patch parse to valid numbers and
patch > 0 (no prerelease suffixes) — if any check fails, call throwErr(...) with
a clear message indicating the bad input; only when validation passes return
`${major}.${minor}.${patch - 1}`.
- Around line 176-177: Add a brief code comment above the lines that set
esmVersion and esmFallbackVersion (the variables esmVersion, esmFallbackVersion
and the call getEsmFallbackVersion(packageJson.version)) documenting the
invariant: explain that the monorepo uses changesets with fixed-versioning
("fixed": [["**"]]) so all packages (e.g., apps/dashboard, `@stackframe/js`,
`@stackframe/dashboard-ui-components`) share the same release version, and
therefore esmVersion will always match a published ESM module version making the
fallback strategy valid. Keep the comment short and placed next to the existing
packageJson.version usage.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx:
- Line 394: The height calculation on the div with data-full-bleed using the
class 'flex h-[calc(100vh-4.5rem)] dark:h-[calc(100vh-5.75rem)]' is fragile;
replace the hardcoded per-theme math by introducing shared CSS variables (e.g.,
--app-header-height-light and --app-header-height-dark) set on :root and .dark
(mirroring the header spacing that currently uses dark:top-3 etc.), then change
the classes to use h-[calc(100vh-var(--app-header-height-light))] and
dark:h-[calc(100vh-var(--app-header-height-dark))] or, even better, move the
viewport calc to a parent flex container so this page component (the div with
data-full-bleed) doesn't need to know header dimensions; also consider switching
to 100dvh in the calc for mobile browser consistency.
In
`@apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx`:
- Around line 30-163: The dev and prod branches in getDependencyScripts
duplicate the same helper functions (formatDependencyError,
reportDependencyError, failDependencyLoad); extract those shared helpers into a
new function getDependencyHelpersScript(userFacingMessage) that returns the
helper-script string, then prepend its output to both the dev and prod module
strings in getDependencyScripts; have dev call getDependencyHelpersScript with a
pass-through/raw message and prod call it with
CUSTOM_DASHBOARD_LOAD_ERROR_MESSAGE so only the message differs while imports
and load logic remain in their respective branches (update references to
formatDependencyError, reportDependencyError, and failDependencyLoad inside both
inline scripts to use the hoisted helpers).
- Around line 802-807: The dependency-error handler currently only calls
captureError('dashboard-sandbox-dependency-error', err) and returns, which
delays and duplicates user-facing error handling; after creating and capturing
the Error (the err built from event.data), also invoke
onRuntimeErrorRef.current?.(err) so the runtime error is surfaced to the parent
immediately (this will be deduped by the parent's lastErrorRef window against
the later stack-ai-dashboard-error), keeping the same Error instance so
waitForDeps / initializeStackApp boot-failure flow still sees the original error
if it rethrows.
🪄 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: aaaa7db5-f920-4c77-a61a-a219af8ec90f
📒 Files selected for processing (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsxapps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx
|
@greptile review again |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx (1)
396-396: ⚡ Quick winUse dynamic viewport units to avoid mobile height glitches.
Line 396 uses
100vh, which can cause clipping/jumps on mobile when browser UI expands/collapses. Prefer100dvhfor the dashboard shell height.Proposed tweak
- <div data-full-bleed className="flex h-[calc(100vh-4.5rem)] dark:h-[calc(100vh-5.75rem)]"> + <div data-full-bleed className="flex h-[calc(100dvh-4.5rem)] dark:h-[calc(100dvh-5.75rem)]">🤖 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/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx at line 396, The dashboard shell uses CSS height with viewport unit "100vh" which can glitch on mobile; update the div with data-full-bleed (the element that currently has className "flex h-[calc(100vh-4.5rem)] dark:h-[calc(100vh-5.75rem)]") to use dynamic viewport units ("100dvh") instead of "100vh" in both the light and dark calc expressions so they become "calc(100dvh - 4.5rem)" and "calc(100dvh - 5.75rem)" respectively.
🤖 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.
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx:
- Line 396: The dashboard shell uses CSS height with viewport unit "100vh" which
can glitch on mobile; update the div with data-full-bleed (the element that
currently has className "flex h-[calc(100vh-4.5rem)]
dark:h-[calc(100vh-5.75rem)]") to use dynamic viewport units ("100dvh") instead
of "100vh" in both the light and dark calc expressions so they become
"calc(100dvh - 4.5rem)" and "calc(100dvh - 5.75rem)" respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60627499-051d-4104-b1d9-2b6ee3bc319f
📒 Files selected for processing (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsxapps/dashboard/src/components/commands/create-dashboard/create-dashboard-preview.tsxapps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx
|
@greptile review again |
This PR fixes the versioning error that we ran into for custom dashboards. Now if the latest version of the packages does not work, we fall back to the version that is one patch below the latest version. We log this into sentry. If the fall back doesn't work either, we log that into sentry as well and show the user an error message.
Apart from that, I also made changes to ensure dashboards with older versions of the dashboard-ui-component package would still work. Each dashboard now stores the version it was created with, as a comment at the top of its source code, and we use that version when loading the dashboard. When a dashboard gets edited via the AI chat, we re-stamp it with the latest version of the package so it stays up to date.
Summary by CodeRabbit
Bug Fixes
New Features
UI/UX Improvements