-
Notifications
You must be signed in to change notification settings - Fork 147
feat(dashboard): use vercel-flavored examples when deploying with vercel #3986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 01-20-feat_examples_add_vercel_examples
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Pull Request ReviewOverviewThis PR updates the Vercel deployment template link to use template-specific Vercel configurations from the example registry, improving the deployment experience by directing users to the correct Vercel-flavored examples. Code Quality: ✅ GoodPositive aspects:
Issues & Concerns1. Potential Bug: Null Safety (Medium Priority)In <DeployToVercelCard template={templateDetails?.providers.vercel.name || template || "chat-room"} />Issue: If Recommendation: <DeployToVercelCard template={templateDetails?.providers?.vercel?.name || template || "chat-room"} />Add optional chaining to 2. Inconsistency: Different Usage Patterns (Low Priority)The
This inconsistency could lead to confusion. Consider:
3. Type Safety Consideration (Low Priority)The lookup pattern Performance: ✅ Good
Security: ✅ No concerns
Test Coverage:
|
9533db4 to
0bf7359
Compare
| @@ -1,5 +1,5 @@ | |||
| import { faChevronLeft, faChevronRight, Icon } from "@rivet-gg/icons"; | |||
| import { deployOptions } from "@rivetkit/example-registry"; | |||
| import { deployOptions, templates } from "@rivetkit/example-registry"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement was modified to include 'templates', but this might have caused import sorting issues. Ensure all imports are properly sorted according to Biome's rules. Run Biome's linter to automatically fix the sorting.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| return useMemo(() => { | ||
| // const repositoryUrl = `https://github.com/rivet-dev/rivet/tree/main/examples/${template || "chat-room"}`; | ||
| const repositoryUrl = "https://github.com/rivet-dev/template-vercel"; | ||
| const repositoryUrl = `https://github.com/rivet-dev/rivet/tree/main/examples/${template || "chat-room"}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useMemo hook is likely missing the 'template' variable in its dependency array. Add template to the dependency array to ensure the memoized value is recalculated when template changes. The correct implementation would be: 'return useMemo(() => { const repositoryUrl = https://github.com/rivet-dev/rivet/tree/main/examples/${template || "chat-room"}; ... }, [template, endpoint]);'
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| const templateDetails = templates.find((t) => t.name === template); | ||
| const options = deployOptions.find((p) => p.name === provider); | ||
| return ( | ||
| <div className="flex flex-col gap-6"> | ||
| {match({ template, provider }) | ||
| .with({ provider: "vercel", template: P.string }, () => ( | ||
| <DeployToVercelCard template={template} /> | ||
| <DeployToVercelCard template={templateDetails?.providers.vercel.name || template || "chat-room"} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is trying to access 'providers.vercel.name' on templateDetails, but this property doesn't exist in the Template type definition. Since we don't have access to modify the Template type, we should remove line 377 that introduces templateDetails and modify line 383 to use the template parameter directly: <DeployToVercelCard template={template || 'chat-room'} />
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.

TL;DR
Updated the Vercel deployment template link to use the correct repository path and improved template handling for Vercel deployments.
What changed?
https://github.com/rivet-dev/template-vercel) with a dynamic path that points to examples in the main Rivet repository (https://github.com/rivet-dev/rivet/tree/main/examples/${template || "chat-room"})templatesfrom@rivetkit/example-registryin the getting-started componentDeployToVercelCardcomponent to use template-specific Vercel provider configuration when available, falling back to the template name or "chat-room" as defaultHow to test?
Why make this change?
This change ensures that users are directed to the correct template repositories when deploying to Vercel, providing a more consistent experience across different templates. It also improves the flexibility of the deployment system by using template-specific Vercel configurations when available.