Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughTwo unrelated changes: a viewport-based height calculation adjustment in the dashboard page client, and an enhanced sandbox dependency-loading flow that adds fallback-version computation, structured dependency-error reporting from iframe to host, and initialization halting on dependency 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 (primary esmVersion)
alt Primary succeeds
Deps-->>Sandbox: SDK loaded
Sandbox->>Sandbox: Set globals, initialize UI
Sandbox-->>Host: deps-ready / ready
else Primary fails
Deps-->>Sandbox: Import error
Sandbox->>Deps: Import fallback (esmFallbackVersion)
alt Fallback succeeds
Deps-->>Sandbox: SDK loaded
Sandbox->>Sandbox: Set globals, initialize UI
Sandbox-->>Host: deps-ready / ready
else Fallback fails
Deps-->>Sandbox: Import error
Sandbox->>Sandbox: Set window.__depsError and format error
Sandbox-->>Host: dashboard-sandbox-dependency-error (with optional stack)
Host->>Host: captureError('dashboard-sandbox-dependency-error', ...)
Host->>Host: Halt initialization
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR improves the custom dashboard sandbox by replacing a stale hardcoded fallback version (
Confidence Score: 5/5Safe to merge; the dependency-error plumbing is well-structured and the fallback guard prevents globals from being set in a failed state. The core logic — structured error channels, No files require special attention beyond the minor dev-path flow noted in the inline comment. Important Files Changed
Sequence DiagramsequenceDiagram
participant IFrame as Sandbox iframe
participant Host as DashboardSandboxHost
participant User as User UI
IFrame->>IFrame: "import @stackframe/js@esmVersion"
alt primary load fails
IFrame->>Host: dashboard-sandbox-dependency-error (technical details)
Host->>Host: captureError (telemetry only)
IFrame->>IFrame: "import @stackframe/js@esmFallbackVersion"
alt fallback load also fails
IFrame->>IFrame: failDependencyLoad set __depsError fire deps-ready
IFrame->>Host: dashboard-sandbox-dependency-error (technical details)
Host->>Host: captureError (telemetry only)
IFrame->>IFrame: waitForDeps() throws user-friendly message in prod
IFrame->>Host: dashboard-error-boundary via unhandledrejection
Host->>User: onRuntimeErrorRef show error UI
else fallback succeeds
IFrame->>IFrame: set window.StackSDK globals fire deps-ready
end
else primary load succeeds
IFrame->>IFrame: set window.StackSDK globals
end
IFrame->>IFrame: load IIFE dashboard-ui-components
alt IIFE loads
IFrame->>IFrame: __depsReady true fire deps-ready
else IIFE fails
IFrame->>IFrame: failDependencyLoad set __depsError fire deps-ready
IFrame->>Host: dashboard-sandbox-dependency-error
Host->>Host: captureError telemetry only
end
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:79-83
In dev mode, when both SDK versions fail `failDependencyLoad` marks deps as failed and fires `deps-ready`, but execution does **not** stop — the code falls through to `window.generateUuid` and appends the IIFE `<script>` element. If the IIFE then loads successfully its `onload` fires `deps-ready` a second time, and if the IIFE also fails its `onerror` calls `failDependencyLoad` a second time, overwriting `window.__depsError` with the less-informative "IIFE bundle missing" message instead of the root-cause SDK error. A `return` after the inner `failDependencyLoad` call stops both spurious effects.
```suggestion
} catch (e2) {
failDependencyLoad('[sandbox] @stackframe/js fallback failed at version ${esmFallbackVersion}: ' + formatDependencyError(e2), e2);
return;
}
}
window.generateUuid = () => crypto.randomUUID();
```
Reviews (2): Last reviewed commit: "Enhance version fallback logic in dashbo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
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.
Comment on lines
+24
to
+26
| const parts = version.split("."); | ||
| const [major, minor, patchString] = parts; | ||
| const patch = Number(patchString); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
Comment on lines
176
to
+177
| const esmVersion = packageJson.version; | ||
| const esmFallbackVersion = "2.8.71"; | ||
| const esmFallbackVersion = getEsmFallbackVersion(esmVersion); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the version-sync assumption: is the dashboard app version always == the SDK package versions?
echo "=== Dashboard app version ==="
jq -r '.version' apps/dashboard/package.json 2>/dev/null
echo
echo "=== `@stackframe/js` package version (in repo) ==="
fd -t f 'package.json' | xargs -I{} sh -c 'jq -e ".name == \"@stackframe/js\"" {} >/dev/null 2>&1 && echo "{}: $(jq -r .version {})"'
echo
echo "=== `@stackframe/dashboard-ui-components` package version (in repo) ==="
fd -t f 'package.json' | xargs -I{} sh -c 'jq -e ".name == \"@stackframe/dashboard-ui-components\"" {} >/dev/null 2>&1 && echo "{}: $(jq -r .version {})"'
echo
echo "=== Release config / changesets coupling ==="
fd -t f -e json -e md . .changeset 2>/dev/null | head -n 20
fd -t f 'release.config*' 2>/dev/null
rg -nP --no-heading -C2 '"version"\s*:' --glob 'package.json' --glob '!**/node_modules/**' | head -n 80Repository: hexclave/stack-auth
Length of output: 5147
🏁 Script executed:
#!/bin/bash
# Understand the release strategy and version coupling
echo "=== Changeset config ==="
cat .changeset/config.json 2>/dev/null | head -40
echo
echo "=== Release config files ==="
find . -maxdepth 2 -name 'release.config*' -o -name '.releaserc*' 2>/dev/null
echo
echo "=== GitHub Actions workflow files related to release/publish ==="
fd -t f '.github/workflows' | xargs -I{} sh -c 'rg -l "release|publish|version" "{}" 2>/dev/null' | head -10
echo
echo "=== Check for monorepo configuration (lerna.json, pnpm-workspace) ==="
ls -la lerna.json pnpm-workspace.yaml package-lock.json yarn.lock 2>/dev/null | grep -v "cannot access"
echo
echo "=== Look at root package.json for workspaces/publishing ==="
jq '.workspaces, .publishConfig, .version' package.json 2>/dev/null
echo
echo "=== Check if dashboard is listed in changesets for independent publishing ==="
rg -i "dashboard" .changeset/ --type md -A2 2>/dev/null | head -30Repository: hexclave/stack-auth
Length of output: 1128
🏁 Script executed:
#!/bin/bash
# Verify the pnpm workspace and release workflow
echo "=== pnpm workspace config ==="
cat pnpm-workspace.yaml
echo
echo "=== GitHub Actions workflows (corrected) ==="
fd . '.github/workflows' -type f | head -20
echo
echo "=== Look for publish/release workflow content ==="
fd . '.github/workflows' -type f | xargs grep -l "publish\|release" 2>/dev/null | head -5 | xargs cat 2>/dev/null | head -100Repository: hexclave/stack-auth
Length of output: 692
🏁 Script executed:
#!/bin/bash
# Find workflows correctly
echo "=== GitHub Actions workflows ==="
find .github/workflows -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -20
echo
echo "=== Publish/Release workflows ==="
find .github/workflows -type f \( -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | xargs grep -l "publish\|release" 2>/dev/null
echo
echo "=== Changesets related config and workflow ==="
find . -maxdepth 3 -name "*changeset*" -o -name "*release*" 2>/dev/null | grep -v node_modules | head -20
echo
echo "=== Inspect the main publish/release workflow if it exists ==="
find .github/workflows -type f \( -name "*.yml" -o -name "*.yaml" \) -exec grep -l changesets {} \; 2>/dev/null | xargs cat 2>/dev/null | head -150Repository: hexclave/stack-auth
Length of output: 1221
🏁 Script executed:
#!/bin/bash
# Examine the npm-publish workflow
echo "=== npm-publish.yaml workflow ==="
cat .github/workflows/npm-publish.yaml
echo
echo "=== Check for any release-please or conventional commits ==="
rg "release-please|conventional" . --type yaml -A3 -B3 2>/dev/null
echo
echo "=== Changeset documentation in repo ==="
find . -name "CHANGELOG*" -o -name "RELEASE*" 2>/dev/null | head -10Repository: hexclave/stack-auth
Length of output: 2047
Add a comment documenting the version-sync invariant.
The version assumption is enforced by the monorepo's changesets configuration, which uses "fixed": [["**"]] to ensure all packages—including apps/dashboard, @stackframe/js, and @stackframe/dashboard-ui-components—must release with the same version number. However, this constraint isn't obvious from the code alone. Add a brief comment near line 176 explaining that the changesets fixed-versioning scheme guarantees that esmVersion will always correspond to a published version of the ESM modules, keeping the fallback strategy effective.
🤖 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 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.
Collaborator
Author
|
@greptile review again |
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
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.
To ensure we dont get into the same problem again, this is what i have done - basically now if the latest version 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 doesnt work either, we log that into sentry as well and show the user an error message.
Summary by CodeRabbit
Bug Fixes
UI/UX Improvements