Update dashboard components (Except for major 4)#1205
Update dashboard components (Except for major 4)#1205Developing-Gamer merged 100 commits intodevfrom
Conversation
…hosphor icons successful
…email logs. Updated sender email formatting to exclude non-alphanumeric characters. Added tests for email template deletion scenarios, including handling of shared email configurations.
…and user feedback. Added state management for delete errors and updated the confirmation dialog to display error messages when deletion fails.
…g. Enhance mobile layout in VibeCodeLayout with responsive design and chat functionality.
… effects and transitions. Added accent hover tint for better visual feedback.
…Removed redundant try-catch block and streamlined message validation process for improved readability and maintainability.
…lobe component to fix flickering bug, improved culling.
- Added dynamic string handling for email template configuration overrides. - Updated line-chart component to use urlString for routing. - Refactored data vault store configuration to directly set displayName. - Improved error handling in email template deletion process. - Enhanced launch checklist with new asynchronous alert functionality. - Updated Vercel page to ensure proper key assignment with error handling. - Modified dropdown menu state initialization for better default handling. - Improved data table component to prevent multiple calls on table readiness.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx (1)
150-156:⚠️ Potential issue | 🔴 CriticalBug:
"prevent-close"return value fromhandleDeleteis silently discarded.The
onClickhandlerawaitshandleDeletebut neverreturns its result, soActionDialogalways seesundefinedand closes the dialog — even when the delete fails. This means the error alert on lines 163-170 is still never visible, despite thereturn "prevent-close"added on line 36.Proposed fix
okButton={{ label: "Delete", onClick: async () => { if (deleteDialogOpen) { - await handleDelete(deleteDialogOpen); + return await handleDelete(deleteDialogOpen); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx around lines 150 - 156, The okButton onClick awaits handleDelete(deleteDialogOpen) but does not return its result, so the "prevent-close" sentinel is discarded and ActionDialog always closes; update the okButton.onClick handler in the component (the okButton block where deleteDialogOpen is checked) to return the awaited result of handleDelete(deleteDialogOpen) (i.e., use "return await handleDelete(deleteDialogOpen);") so ActionDialog receives the "prevent-close" value and can keep the dialog open on failure.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx (1)
436-457:⚠️ Potential issue | 🔴 CriticalBug: error list never renders — condition is always true.
productionChecksPassingis defined asproductionModeErrors.length === 0(line 418), so the guard on line 437 (productionChecksPassing || productionModeErrors.length === 0) is tautologically true. Theelsebranch (lines 441–456) showing the list of production-mode errors is dead code — users will never see what they need to fix.The intent was likely
!productionChecksPassingas the first operand (or simply checkingproductionModeErrors.length === 0alone):Proposed fix
- productionChecksPassing || productionModeErrors.length === 0 ? ( + productionModeErrors.length === 0 ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx around lines 436 - 457, The conditional always evaluates true because productionChecksPassing is defined as productionModeErrors.length === 0; update the JSX guard so the error list renders when checks fail by using the negation (e.g., !productionChecksPassing) or directly checking productionModeErrors.length > 0 in the ternary condition where productionChecksPassing || productionModeErrors.length === 0 is currently used; modify the conditional around productionChecksPassing/productionModeErrors in the JSX block that renders the "All checks are passing." vs the error list so the else branch with productionModeErrors.map(...) can execute when there are errors.apps/dashboard/src/components/ui/dropdown-menu.tsx (1)
119-152:⚠️ Potential issue | 🔴 Critical
onClickleaks through the{...props}spread, still causing double execution on mouse clicks.
onClickis not destructured out of...propson line 119, so{...props}on line 150 passes it directly toDropdownMenuPrimitive.Item. On a mouse click, the nativeonClickfiresprops.onClickdirectly andonSelectfireshandleItemActionwhich callsprops.onClickagain — the same double-execution bug from the previous review, just via a different path.Destructure
onClickout of the rest props so it doesn't leak to the primitive:Proposed fix
->(({ className, inset, icon, ...props }, ref) => { +>(({ className, inset, icon, onClick, ...props }, ref) => { const { setOpen } = React.useContext(DropdownMenuContext) ?? throwErr("No DropdownMenuContext found"); const [isLoading, setIsLoading] = React.useState(false); // Share activation logic so keyboard onSelect matches mouse clicks. const handleItemAction = (event: { preventDefault: () => void, stopPropagation: () => void }) => { event.preventDefault(); event.stopPropagation(); - const result = props.onClick?.(event as unknown as React.MouseEvent<HTMLDivElement, MouseEvent>); + const result = onClick?.(event as unknown as React.MouseEvent<HTMLDivElement, MouseEvent>); if (result instanceof Promise) { setIsLoading(true); runAsynchronouslyWithAlert( result.finally(() => { setIsLoading(false); setOpen(false); }) ); } else { setOpen(false); } }; ... {...props} disabled={isLoading || props.disabled} - onSelect={props.onClick ? handleItemAction : undefined} + onSelect={onClick ? handleItemAction : undefined} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ui/dropdown-menu.tsx` around lines 119 - 152, The onClick handler is leaking through the {...props} spread and causing double execution (native onClick plus onSelect -> handleItemAction -> props.onClick); fix by destructuring onClick from the component args (e.g., change ({ className, inset, icon, ...props }, ref) to extract onClick: const { onClick, ...rest } or destructure directly) and use rest in the JSX spread so DropdownMenuPrimitive.Item does not receive onClick, then reference the extracted onClick inside handleItemAction and set onSelect={onClick ? handleItemAction : undefined} and disabled based on isLoading || rest.disabled. Ensure you update all uses of props to rest and props.onClick to the extracted onClick (handleItemAction should call the extracted onClick).apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/vercel/page-client.tsx (1)
63-63:⚠️ Potential issue | 🟡 MinorDead code:
errorstate is declared but never set.
setError(line 63) is never called anywhere — the oldcatchblock that presumably calledsetError(...)appears to have been removed whenrunAsynchronouslyWithAlertwas adopted. As a result,props.errorinStepCardis alwaysnulland theDesignAlerton lines 591-595 will never render. Either remove the deaderrorstate and related UI, or restore error-setting logic if per-field error display is still desired alongside the global alert.Option A: Remove dead code
- const [error, setError] = useState<string | null>(null);And remove the
errorprop fromStepCardusage and its rendering logic.Also applies to: 590-595
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/vercel/page-client.tsx at line 63, The local state error declared as `const [error, setError] = useState<string | null>(null)` is dead because `setError` is never called after switching to `runAsynchronouslyWithAlert`; either remove this state and all uses (remove the `error` prop passed into `StepCard` and the `DesignAlert` rendering around lines where `props.error` is referenced), or restore error-setting where failures are handled by calling `setError(...)` inside the async handlers invoked by `runAsynchronouslyWithAlert` so `StepCard` and `DesignAlert` receive and display per-field errors; update the `StepCard` prop usage and any conditional rendering accordingly (refer to `error`, `setError`, `runAsynchronouslyWithAlert`, `StepCard`, and `DesignAlert` to locate the changes).
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/(overview)/line-chart.tsx:
- Around line 186-218: Hoist the static mapping const designCardGradients out of
the component and into module scope so it isn't recreated on every render: move
the existing designCardGradients declaration to the top-level of the file (above
the component) and leave all references (designCardGradients[gradientColor])
inside the component unchanged; ensure the identifier name remains the same and
adjust any imports/exports only if you intend to reuse it across files.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx:
- Line 47: Replace the plain template-literal URL construction used in the
DesignButton onClick (router.push(`/projects/${project.id}/data-vault/stores`))
and the other router.push that uses
`/projects/${project.id}/data-vault/stores/${storeId}` with the urlString
template tag (e.g., urlString`/projects/${project.id}/data-vault/stores` and
urlString`/projects/${project.id}/data-vault/stores/${storeId}`) and add the
missing import for urlString at the top of the file so special characters in
project.id or storeId are properly encoded; update the two router.push call
sites (the DesignButton onClick and the other redirect using storeId) to use
urlString instead of plain interpolation.
- Around line 110-117: The onClick currently calls copyToClipboard(storeId) and
drops the returned promise; wrap that async call with the project's async error
handler so clipboard failures aren't swallowed—replace the arrow handler on the
DesignButton with a call to runAsynchronously or runAsynchronouslyWithAlert
(import it if missing) and pass a callback that invokes
copyToClipboard(storeId); keep DesignButton, copyToClipboard, and the chosen
runAsynchronously/runAsynchronouslyWithAlert identifiers to locate and update
the handler.
- Around line 36-54: The code reads `store = config.dataVault.stores[storeId]`
before checking existence and declares `modifiedKeys` with `useMemo`, which can
leave `store` typed as possibly undefined later; fix by either moving the
existence guard (the `if (!(storeId in config.dataVault.stores))` block) above
the `store` and `useMemo` declarations so hooks remain in the same order across
returns, or keep the current order and add an explicit defensive narrowing
immediately after the guard (e.g., throw via a utility if `store` is undefined)
so usages later (references to `store`, `modifiedKeys`, `useMemo`) see a
non-undefined `store`; ensure you do not introduce conditional hooks when
reordering.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-templates/page-client.tsx:
- Around line 150-156: The okButton onClick awaits
handleDelete(deleteDialogOpen) but does not return its result, so the
"prevent-close" sentinel is discarded and ActionDialog always closes; update the
okButton.onClick handler in the component (the okButton block where
deleteDialogOpen is checked) to return the awaited result of
handleDelete(deleteDialogOpen) (i.e., use "return await
handleDelete(deleteDialogOpen);") so ActionDialog receives the "prevent-close"
value and can keep the dialog open on failure.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx:
- Around line 539-548: The confetti timer uses Date.now() for elapsed-time
calculations; update the logic to use performance.now() instead by setting
animationEnd based on performance.now() + duration (replace the current
animationEnd = Date.now() + duration) and compute timeLeft as animationEnd -
performance.now() inside the setInterval callback (keep the rest of the confetti
logic including defaults, randomInRange, and interval unchanged). Ensure all
references to Date.now() in this block are replaced so elapsed-time measurement
consistently uses performance.now().
- Around line 149-158: The cardClass values for the checklist states (keys done,
action, blocked) use hover-enter classes like "transition-all duration-300
hover:shadow-lg"; update each cardClass in page-client.tsx to follow the
dashboard hover-exit pattern by moving the transition to the base state and
removing hover-enter effects—specifically, keep a base transition (e.g.,
transition-[property] and duration) and replace hover:shadow-lg with
hover:transition-none (and remove or neutralize any shadow-on-hover) so the
visual transition occurs on hover-exit rather than on hover-enter; update the
cardClass strings for done, action, and blocked accordingly.
- Around line 142-161: STATUS_META currently defines identical cardClass strings
for all LaunchTaskStatus keys (done, action, blocked); either deduplicate by
extracting a shared constant and reusing it in STATUS_META or assign distinct
cardClass values per status if visual differentiation is intended. Update the
STATUS_META declaration (and the LaunchTaskStatus usages) to reference a single
constant like DEFAULT_CARD_CLASS for the common string, or replace the three
identical cardClass values with unique style strings for done/action/blocked so
the per-status lookup becomes meaningful.
- Around line 436-457: The conditional always evaluates true because
productionChecksPassing is defined as productionModeErrors.length === 0; update
the JSX guard so the error list renders when checks fail by using the negation
(e.g., !productionChecksPassing) or directly checking
productionModeErrors.length > 0 in the ternary condition where
productionChecksPassing || productionModeErrors.length === 0 is currently used;
modify the conditional around productionChecksPassing/productionModeErrors in
the JSX block that renders the "All checks are passing." vs the error list so
the else branch with productionModeErrors.map(...) can execute when there are
errors.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/vercel/page-client.tsx:
- Around line 403-414: The subtitle prop on the DesignCard is using the HTML
entity ' which renders literally in JSX attributes; update the subtitle
value (on the DesignCard component instance) to use a real apostrophe or a JS
string expression instead (e.g. subtitle="See Vercel's documentation on
environment variables for more details." or subtitle={'See Vercel\\'s
documentation on environment variables for more details.'}) so the apostrophe
displays correctly.
- Around line 492-496: The DesignButton/StyledLink combination currently uses
"transition-all duration-150" which triggers hover-enter transitions; replace
that with a hover-exit pattern such as "transition-colors duration-150
hover:transition-none" (keeping other classes like "font-medium border
border-border shadow-sm active:scale-95 dark:..." intact) for the instance
inside page-client.tsx and the matching button(s) later in the file to follow
the dashboard guideline.
- Line 63: The local state error declared as `const [error, setError] =
useState<string | null>(null)` is dead because `setError` is never called after
switching to `runAsynchronouslyWithAlert`; either remove this state and all uses
(remove the `error` prop passed into `StepCard` and the `DesignAlert` rendering
around lines where `props.error` is referenced), or restore error-setting where
failures are handled by calling `setError(...)` inside the async handlers
invoked by `runAsynchronouslyWithAlert` so `StepCard` and `DesignAlert` receive
and display per-field errors; update the `StepCard` prop usage and any
conditional rendering accordingly (refer to `error`, `setError`,
`runAsynchronouslyWithAlert`, `StepCard`, and `DesignAlert` to locate the
changes).
In `@apps/dashboard/src/components/ui/dropdown-menu.tsx`:
- Around line 119-152: The onClick handler is leaking through the {...props}
spread and causing double execution (native onClick plus onSelect ->
handleItemAction -> props.onClick); fix by destructuring onClick from the
component args (e.g., change ({ className, inset, icon, ...props }, ref) to
extract onClick: const { onClick, ...rest } or destructure directly) and use
rest in the JSX spread so DropdownMenuPrimitive.Item does not receive onClick,
then reference the extracted onClick inside handleItemAction and set
onSelect={onClick ? handleItemAction : undefined} and disabled based on
isLoading || rest.disabled. Ensure you update all uses of props to rest and
props.onClick to the extracted onClick (handleItemAction should call the
extracted onClick).
🧹 Nitpick comments (6)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/(overview)/line-chart.tsx: - Around line 186-218: Hoist the static mapping const designCardGradients out of the component and into module scope so it isn't recreated on every render: move the existing designCardGradients declaration to the top-level of the file (above the component) and leave all references (designCardGradients[gradientColor]) inside the component unchanged; ensure the identifier name remains the same and adjust any imports/exports only if you intend to reuse it across files. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx: - Around line 36-54: The code reads `store = config.dataVault.stores[storeId]` before checking existence and declares `modifiedKeys` with `useMemo`, which can leave `store` typed as possibly undefined later; fix by either moving the existence guard (the `if (!(storeId in config.dataVault.stores))` block) above the `store` and `useMemo` declarations so hooks remain in the same order across returns, or keep the current order and add an explicit defensive narrowing immediately after the guard (e.g., throw via a utility if `store` is undefined) so usages later (references to `store`, `modifiedKeys`, `useMemo`) see a non-undefined `store`; ensure you do not introduce conditional hooks when reordering. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx: - Around line 539-548: The confetti timer uses Date.now() for elapsed-time calculations; update the logic to use performance.now() instead by setting animationEnd based on performance.now() + duration (replace the current animationEnd = Date.now() + duration) and compute timeLeft as animationEnd - performance.now() inside the setInterval callback (keep the rest of the confetti logic including defaults, randomInRange, and interval unchanged). Ensure all references to Date.now() in this block are replaced so elapsed-time measurement consistently uses performance.now(). - Around line 149-158: The cardClass values for the checklist states (keys done, action, blocked) use hover-enter classes like "transition-all duration-300 hover:shadow-lg"; update each cardClass in page-client.tsx to follow the dashboard hover-exit pattern by moving the transition to the base state and removing hover-enter effects—specifically, keep a base transition (e.g., transition-[property] and duration) and replace hover:shadow-lg with hover:transition-none (and remove or neutralize any shadow-on-hover) so the visual transition occurs on hover-exit rather than on hover-enter; update the cardClass strings for done, action, and blocked accordingly. - Around line 142-161: STATUS_META currently defines identical cardClass strings for all LaunchTaskStatus keys (done, action, blocked); either deduplicate by extracting a shared constant and reusing it in STATUS_META or assign distinct cardClass values per status if visual differentiation is intended. Update the STATUS_META declaration (and the LaunchTaskStatus usages) to reference a single constant like DEFAULT_CARD_CLASS for the common string, or replace the three identical cardClass values with unique style strings for done/action/blocked so the per-status lookup becomes meaningful. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/vercel/page-client.tsx: - Around line 492-496: The DesignButton/StyledLink combination currently uses "transition-all duration-150" which triggers hover-enter transitions; replace that with a hover-exit pattern such as "transition-colors duration-150 hover:transition-none" (keeping other classes like "font-medium border border-border shadow-sm active:scale-95 dark:..." intact) for the instance inside page-client.tsx and the matching button(s) later in the file to follow the dashboard guideline.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx (3)
539-548: Useperformance.now()instead ofDate.now()for elapsed-time measurement.The confetti animation interval computes remaining time via
Date.now(). Per coding guidelines,performance.now()should be used for measuring elapsed (real) time.Proposed fix
- const animationEnd = Date.now() + duration; + const animationEnd = performance.now() + duration; ... - const timeLeft = animationEnd - Date.now(); + const timeLeft = animationEnd - performance.now();As per coding guidelines: "Use
performance.now()instead ofDate.now()for measuring elapsed (real) time."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx around lines 539 - 548, The confetti timer uses Date.now() for elapsed-time calculations; update the logic to use performance.now() instead by setting animationEnd based on performance.now() + duration (replace the current animationEnd = Date.now() + duration) and compute timeLeft as animationEnd - performance.now() inside the setInterval callback (keep the rest of the confetti logic including defaults, randomInRange, and interval unchanged). Ensure all references to Date.now() in this block are replaced so elapsed-time measurement consistently uses performance.now().
149-158: Consider hover-exit transition pattern per dashboard guidelines.Classes like
transition-all duration-300 hover:shadow-lgare hover-enter transitions. The dashboard guideline recommends the inverse: apply transitions on exit, not entry (e.g.,transition-all hover:transition-none), so the visual effect feels snappier.As per coding guidelines: "Use hover-exit transitions instead of hover-enter transitions. For example,
transition-colors hover:transition-none."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx around lines 149 - 158, The cardClass values for the checklist states (keys done, action, blocked) use hover-enter classes like "transition-all duration-300 hover:shadow-lg"; update each cardClass in page-client.tsx to follow the dashboard hover-exit pattern by moving the transition to the base state and removing hover-enter effects—specifically, keep a base transition (e.g., transition-[property] and duration) and replace hover:shadow-lg with hover:transition-none (and remove or neutralize any shadow-on-hover) so the visual transition occurs on hover-exit rather than on hover-enter; update the cardClass strings for done, action, and blocked accordingly.
142-161: All threeSTATUS_METAentries share the samecardClassstring.The
done,action, andblockedstatuses have identicalcardClassvalues, which makes the per-status lookup pointless. If differentiation is planned, consider adding distinct styles; otherwise, extract a single constant to avoid the repetition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/launch-checklist/page-client.tsx around lines 142 - 161, STATUS_META currently defines identical cardClass strings for all LaunchTaskStatus keys (done, action, blocked); either deduplicate by extracting a shared constant and reusing it in STATUS_META or assign distinct cardClass values per status if visual differentiation is intended. Update the STATUS_META declaration (and the LaunchTaskStatus usages) to reference a single constant like DEFAULT_CARD_CLASS for the common string, or replace the three identical cardClass values with unique style strings for done/action/blocked so the per-status lookup becomes meaningful.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/vercel/page-client.tsx (1)
492-496: Prefer hover-exit transitions per dashboard guidelines.The buttons use
transition-all duration-150which applies transitions on hover-enter. The coding guideline states: "Use hover-exit transitions instead of hover-enter transitions. For example,transition-colors hover:transition-none."Proposed fix
- className="font-medium border border-border shadow-sm transition-all duration-150 hover:bg-accent active:scale-95 ..." + className="font-medium border border-border shadow-sm transition-all duration-150 hover:transition-none hover:bg-accent active:scale-95 ..."Also applies to: 501-508
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/vercel/page-client.tsx around lines 492 - 496, The DesignButton/StyledLink combination currently uses "transition-all duration-150" which triggers hover-enter transitions; replace that with a hover-exit pattern such as "transition-colors duration-150 hover:transition-none" (keeping other classes like "font-medium border border-border shadow-sm active:scale-95 dark:..." intact) for the instance inside page-client.tsx and the matching button(s) later in the file to follow the dashboard guideline.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx (1)
36-54:storeis read before the existence guard — consider reordering or adding a defensive throw.
storeis assigned on line 36 but may beundefinedifstoreIdisn't present. The early return on line 41 prevents further usage at runtime, but TypeScript may not narrow the type, leavingstoretyped as potentiallyundefinedwhen used later (lines 57, 65, 129). A small reorder makes the intent explicit:♻️ Suggested reorder
const config = project.useConfig(); - const store = config.dataVault.stores[storeId]; - const modifiedKeys = useMemo(() => new Set([ - ...(localDisplayName !== undefined ? ["display-name"] : []), - ]), [localDisplayName]); if (!(storeId in config.dataVault.stores)) { return ( ... ); } + const store = config.dataVault.stores[storeId] ?? throwErr(`Store ${storeId} not found despite passing existence check`); + const modifiedKeys = useMemo(() => new Set([ + ...(localDisplayName !== undefined ? ["display-name"] : []), + ]), [localDisplayName]);Note: this requires that no React hooks are called between the early return and the rest of the component (hooks must not be called conditionally). Since
useMemois currently before the guard, moving it after the guard would violate the rules of hooks if the guard triggers a different return path. This needs careful handling — you may instead keep the current structure and just add an explicit narrowing assertion after the guard.As per coding guidelines, 'Code defensively. Prefer
?? throwErr(...)over non-null assertions, with good error messages explicitly stating the assumption that must've been violated'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/data-vault/stores/[storeId]/page-client.tsx around lines 36 - 54, The code reads `store = config.dataVault.stores[storeId]` before checking existence and declares `modifiedKeys` with `useMemo`, which can leave `store` typed as possibly undefined later; fix by either moving the existence guard (the `if (!(storeId in config.dataVault.stores))` block) above the `store` and `useMemo` declarations so hooks remain in the same order across returns, or keep the current order and add an explicit defensive narrowing immediately after the guard (e.g., throw via a utility if `store` is undefined) so usages later (references to `store`, `modifiedKeys`, `useMemo`) see a non-undefined `store`; ensure you do not introduce conditional hooks when reordering.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/(overview)/line-chart.tsx (1)
186-218: Consider hoistingdesignCardGradientsto module scope.This static mapping doesn't depend on props or state, yet it's recreated on every render of
ChartCard. Moving it outside the component is a small clarity and allocation win.♻️ Proposed change
+const designCardGradients: Record<GradientColor, "blue" | "cyan" | "purple" | "green" | "orange" | "default"> = { + blue: "blue", + purple: "purple", + green: "green", + orange: "orange", + slate: "default", + cyan: "cyan", +}; + export function ChartCard({ children, className, gradientColor = "blue" }: { children: React.ReactNode, className?: string, gradientColor?: GradientColor, }) { - const designCardGradients: Record<GradientColor, "blue" | "cyan" | "purple" | "green" | "orange" | "default"> = { - blue: "blue", - purple: "purple", - green: "green", - orange: "orange", - slate: "default", - cyan: "cyan", - };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/(overview)/line-chart.tsx around lines 186 - 218, Hoist the static mapping const designCardGradients out of the component and into module scope so it isn't recreated on every render: move the existing designCardGradients declaration to the top-level of the file (above the component) and leave all references (designCardGradients[gradientColor]) inside the component unchanged; ensure the identifier name remains the same and adjust any imports/exports only if you intend to reuse it across files.
- Removed default values from the project details dialog in the project settings page. - Added a globe icon to the endpoints design card in the webhooks page for improved UI clarity.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx (1)
88-101: 🛠️ Refactor suggestion | 🟠 MajorURL constructed via plain string interpolation — use
urlStringorencodeURIComponent.
project.idis interpolated directly into the URL without encoding. Per coding guidelines, URLs should useurlString\`orencodeURIComponent()` to prevent issues with special characters.Proposed fix
const jwksUrl = useMemo( - () => `${baseApiUrl}/api/v1/projects/${project.id}/.well-known/jwks.json`, + () => `${baseApiUrl}/api/v1/projects/${encodeURIComponent(project.id)}/.well-known/jwks.json`, [baseApiUrl, project.id] );As per coding guidelines: "Use urlString`` or encodeURIComponent() instead of normal string interpolation for URLs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx around lines 88 - 101, The URL construction currently interpolates project.id directly in jwksUrl (and then used by restrictedJwksUrl and allJwksUrl); update these to encode the project id or use the urlString template helper: build jwksUrl using either urlString`...${encodeURIComponent(project.id)}...` or urlString`...${project.id}...` (where urlString handles encoding), then derive restrictedJwksUrl and allJwksUrl from that encoded jwksUrl to ensure special characters in project.id are safely encoded.
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx:
- Around line 270-278: SmartFormDialog is missing the defaultValues prop so the
form opens blank; pass the current project values as defaultValues (e.g. map
project.displayName and project.description to the schema field names) when
rendering SmartFormDialog, ensuring you use the same field keys used in
projectInformationSchema and update the defaults when the project data changes
(useMemo or derive from project before passing into SmartFormDialog). Keep the
existing props: open={isProjectDetailsDialogOpen},
onOpenChange={setIsProjectDetailsDialogOpen},
formSchema={projectInformationSchema}, and onSubmit={handleProjectDetailsSubmit}
while adding defaultValues to pre-populate the form.
- Line 263: The rendering uses a boolean fallback that treats empty strings as
missing; change the fallback for project.description to an explicit nullish
check (e.g., use the null/undefined check on project.description or the nullish
coalescing behavior) so that an empty string remains displayed while only
null/undefined becomes "-"; update the JSX that references project.description
in page-client.tsx (the <p> that shows project.description) to use an explicit
check like project.description == null ? "-" : project.description or the
equivalent nullish-coalescing approach.
- Line 30: Replace the three uses of `any` with proper types or a short
justification comment: for the `TeamMemberItem` prop `member` use the SDK type
(e.g., `TeamMember`) instead of `any`; for the `team` variable use the SDK's
`Team` type; for the form `values` replace `any` with `yup.InferType<typeof
projectInformationSchema>` (or the explicit inferred type) — if a precise type
truly cannot be expressed, add a one-line comment next to each usage explaining
why `any` is required and reference the limitation (e.g., runtime shape,
third‑party SDK looseness) so the rationale is recorded.
- Around line 303-321: The dark-mode LogoUpload handlers are inline async
lambdas while the light-mode handlers use useCallback (handleLogoChange,
handleFullLogoChange); make the dark-mode handlers consistent by creating
memoized callbacks (e.g., handleLogoDarkChange and handleFullLogoDarkChange)
using useCallback that call project.update({ logoDarkModeUrl }) and
project.update({ logoFullDarkModeUrl }) respectively, and pass those memoized
functions to the LogoUpload onValueChange props so all four handlers are
implemented consistently.
- Around line 88-101: The URL construction currently interpolates project.id
directly in jwksUrl (and then used by restrictedJwksUrl and allJwksUrl); update
these to encode the project id or use the urlString template helper: build
jwksUrl using either urlString`...${encodeURIComponent(project.id)}...` or
urlString`...${project.id}...` (where urlString handles encoding), then derive
restrictedJwksUrl and allJwksUrl from that encoded jwksUrl to ensure special
characters in project.id are safely encoded.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx:
- Around line 415-425: The status column currently hardcodes "Active"; update
the Endpoint type (the type alias used for rows, extending Svix's EndpointOut)
to include a disabled: boolean field, then change the column with id "status"
(the cell renderer) to read the row's disabled value (e.g.,
row.original.disabled) and render DesignBadge with label "Disabled" when true
and "Active" when false, using an appropriate color (e.g., red for disabled,
green for active) and keeping size "sm".
- Around line 396-439: The columns array (ColumnDef<Endpoint> assigned to
columns) is recreated on every render and closes over props.updateFn and
props.onTestRequested causing unnecessary re-renders of DesignDataTable; fix by
moving the columns definition above the early return and wrapping it in useMemo,
e.g. useMemo(() => [ ...columns... ], [props.updateFn, props.onTestRequested])
so ActionMenu receives stable updateFn/onTestEndpoint references and the table
won’t re-render unnecessarily.
- Around line 344-350: The conditional rendering uses a truthy check for the
variable errorMessage; change it to an explicit null/undefined check (e.g.,
errorMessage != null) so the condition only triggers when errorMessage is not
null/undefined; update the JSX around the status, errorMessage, and DesignAlert
usage in the page-client component to use the explicit check.
🧹 Nitpick comments (5)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx: - Line 263: The rendering uses a boolean fallback that treats empty strings as missing; change the fallback for project.description to an explicit nullish check (e.g., use the null/undefined check on project.description or the nullish coalescing behavior) so that an empty string remains displayed while only null/undefined becomes "-"; update the JSX that references project.description in page-client.tsx (the <p> that shows project.description) to use an explicit check like project.description == null ? "-" : project.description or the equivalent nullish-coalescing approach. - Line 30: Replace the three uses of `any` with proper types or a short justification comment: for the `TeamMemberItem` prop `member` use the SDK type (e.g., `TeamMember`) instead of `any`; for the `team` variable use the SDK's `Team` type; for the form `values` replace `any` with `yup.InferType<typeof projectInformationSchema>` (or the explicit inferred type) — if a precise type truly cannot be expressed, add a one-line comment next to each usage explaining why `any` is required and reference the limitation (e.g., runtime shape, third‑party SDK looseness) so the rationale is recorded. - Around line 303-321: The dark-mode LogoUpload handlers are inline async lambdas while the light-mode handlers use useCallback (handleLogoChange, handleFullLogoChange); make the dark-mode handlers consistent by creating memoized callbacks (e.g., handleLogoDarkChange and handleFullLogoDarkChange) using useCallback that call project.update({ logoDarkModeUrl }) and project.update({ logoFullDarkModeUrl }) respectively, and pass those memoized functions to the LogoUpload onValueChange props so all four handlers are implemented consistently. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx: - Around line 396-439: The columns array (ColumnDef<Endpoint> assigned to columns) is recreated on every render and closes over props.updateFn and props.onTestRequested causing unnecessary re-renders of DesignDataTable; fix by moving the columns definition above the early return and wrapping it in useMemo, e.g. useMemo(() => [ ...columns... ], [props.updateFn, props.onTestRequested]) so ActionMenu receives stable updateFn/onTestEndpoint references and the table won’t re-render unnecessarily. - Around line 344-350: The conditional rendering uses a truthy check for the variable errorMessage; change it to an explicit null/undefined check (e.g., errorMessage != null) so the condition only triggers when errorMessage is not null/undefined; update the JSX around the status, errorMessage, and DesignAlert usage in the page-client component to use the explicit check.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx (3)
263-263: Boolean check onproject.descriptionhides empty strings.
project.description || "-"will display"-"for bothnull/undefinedand an empty string"". If an empty description is a valid state distinct from "no description", prefer an explicit nullish check.- <p className="text-sm text-foreground/80">{project.description || "-"}</p> + <p className="text-sm text-foreground/80">{project.description ?? "-"}</p>As per coding guidelines: "Prefer explicit null/undefinedness checks over boolean checks, eg.
foo == nullinstead of!foo".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx at line 263, The rendering uses a boolean fallback that treats empty strings as missing; change the fallback for project.description to an explicit nullish check (e.g., use the null/undefined check on project.description or the nullish coalescing behavior) so that an empty string remains displayed while only null/undefined becomes "-"; update the JSX that references project.description in page-client.tsx (the <p> that shows project.description) to use an explicit check like project.description == null ? "-" : project.description or the equivalent nullish-coalescing approach.
30-30:anytypes used without justification comments.Three places use
anywithout explaining why the type system can't express the correct type:
- Line 30:
member: any- Line 162:
team: any- Line 167:
values: anyPer coding guidelines, when
anyis necessary, a comment should explain why. Ideally, type these properly (e.g., use the SDK'sTeamMember/Teamtypes andyup.InferType<typeof projectInformationSchema>for values).As per coding guidelines: "Try to avoid the
anytype. When necessary, leave a comment explaining why you're using it and why the type system fails there".Also applies to: 162-164, 167-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx at line 30, Replace the three uses of `any` with proper types or a short justification comment: for the `TeamMemberItem` prop `member` use the SDK type (e.g., `TeamMember`) instead of `any`; for the `team` variable use the SDK's `Team` type; for the form `values` replace `any` with `yup.InferType<typeof projectInformationSchema>` (or the explicit inferred type) — if a precise type truly cannot be expressed, add a one-line comment next to each usage explaining why `any` is required and reference the limitation (e.g., runtime shape, third‑party SDK looseness) so the rationale is recorded.
303-321: Dark-mode logo callbacks are inline while light-mode equivalents are memoized.
handleLogoChangeandhandleFullLogoChangeare wrapped inuseCallback(lines 148–154), but the dark-mode variants at lines 306–308 and 316–318 use inline async lambdas. This is an inconsistency — either memoize all four or none.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx around lines 303 - 321, The dark-mode LogoUpload handlers are inline async lambdas while the light-mode handlers use useCallback (handleLogoChange, handleFullLogoChange); make the dark-mode handlers consistent by creating memoized callbacks (e.g., handleLogoDarkChange and handleFullLogoDarkChange) using useCallback that call project.update({ logoDarkModeUrl }) and project.update({ logoFullDarkModeUrl }) respectively, and pass those memoized functions to the LogoUpload onValueChange props so all four handlers are implemented consistently.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx (2)
396-439: Column definitions are recreated on every render.
columnsis defined inline in the render path and capturesprops.updateFn/props.onTestRequestedvia closures. Consider wrapping it inuseMemoto avoid unnecessary re-renders of DesignDataTable.Wrap in useMemo
Move the column definition before the early return and memoize it:
function Endpoints(props: { updateFn: () => void, onTestRequested: (endpoint: Endpoint) => void }) { const endpoints = getSvixResult(useEndpoints({ limit: 100 })); + + const columns: ColumnDef<Endpoint>[] = useMemo(() => [ + // ...column defs unchanged... + ], [props.updateFn, props.onTestRequested]); if (!endpoints.loaded) { return endpoints.rendered; - } else { - const columns: ColumnDef<Endpoint>[] = [ - // ... - ]; - return ( ... ); - } + } + + return ( ... ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx around lines 396 - 439, The columns array (ColumnDef<Endpoint> assigned to columns) is recreated on every render and closes over props.updateFn and props.onTestRequested causing unnecessary re-renders of DesignDataTable; fix by moving the columns definition above the early return and wrapping it in useMemo, e.g. useMemo(() => [ ...columns... ], [props.updateFn, props.onTestRequested]) so ActionMenu receives stable updateFn/onTestEndpoint references and the table won’t re-render unnecessarily.
344-350: Prefer explicit null check forerrorMessage.Line 344 uses a truthy check on
errorMessage(astring | null). Per project guidelines, prefer explicit null/undefinedness checks.- {status === 'error' && errorMessage && ( + {status === 'error' && errorMessage != null && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx around lines 344 - 350, The conditional rendering uses a truthy check for the variable errorMessage; change it to an explicit null/undefined check (e.g., errorMessage != null) so the condition only triggers when errorMessage is not null/undefined; update the JSX around the status, errorMessage, and DesignAlert usage in the page-client component to use the explicit check.
- Introduced default values for project details in the project settings dialog. - Added a 'disabled' state to webhooks endpoints, improving status visibility in the UI. - Updated the SmartForm component to support default values and handle form resets more effectively.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx (2)
289-300:⚠️ Potential issue | 🟠 Major
sendTestEventcan throw unhandled ifsendTestWebhookrejects.If
stackAdminApp.sendTestWebhookthrows a network error or unexpected exception, the status will remain stuck at'sending'and the user will see no feedback. Consider wrapping in try/catch to set the error state on unexpected failures.Suggested fix
const sendTestEvent = async () => { setStatus('sending'); setErrorMessage(null); + try { const result = await stackAdminApp.sendTestWebhook({ endpointId: props.endpoint.id }); if (result.status === 'ok') { setStatus('success'); return; } setStatus('error'); setErrorMessage(result.error.errorMessage); + } catch (e) { + setStatus('error'); + setErrorMessage(e instanceof Error ? e.message : 'An unexpected error occurred'); + } };Note: the outer
ActionDialogonClickmay catch and display the error, but the local state (status) would still be stuck at'sending', leaving the Cancel button hidden (Line 319) and the UI in a broken state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx around lines 289 - 300, sendTestEvent can throw if stackAdminApp.sendTestWebhook rejects; wrap the await call in a try/catch so any thrown error sets local UI state (call setStatus('error') and setErrorMessage with the caught error message or a fallback) and avoid leaving the status stuck at 'sending'. In the catch block reference stackAdminApp.sendTestWebhook failure, call setStatus('error'), setErrorMessage(error?.message ?? 'Unexpected error'), and return (or ensure finally behavior) so the Cancel button and UI are restored.
381-381:⚠️ Potential issue | 🟡 MinorImport and use
urlStringtemplate tag for the route construction.Per coding guidelines, prefer
urlStringorencodeURIComponent()over plain template literals for URL interpolation. TheurlStringutility is available from@stackframe/stack-shared/dist/utils/urlsand is already used throughout the dashboard for similar navigation patterns.Suggested change
+import { urlString } from "@stackframe/stack-shared/dist/utils/urls"; + - { item: "View Details", onClick: () => router.push(`/projects/${project.id}/webhooks/${props.endpoint.id}`) }, + { item: "View Details", onClick: () => router.push(urlString`/projects/${project.id}/webhooks/${props.endpoint.id}`) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx at line 381, The route is built with a plain template literal; import the urlString template tag from `@stackframe/stack-shared/dist/utils/urls` and use it when calling router.push so path segments are properly encoded. Replace router.push(`/projects/${project.id}/webhooks/${props.endpoint.id}`) with router.push(urlString`/projects/${project.id}/webhooks/${props.endpoint.id}`) and add the corresponding import for urlString at the top of the module; reference symbols: router.push, project.id, props.endpoint.id, and urlString.
🧹 Nitpick comments (8)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx (2)
463-468: Inline empty-array literals ([]) create new references each render.
defaultColumnFilters={[]}anddefaultSorting={[]}produce fresh arrays every render. IfDesignDataTableuses these in dependency arrays or passes them touseReactTableoptions, this can trigger unnecessary re-computations. Hoist them to module-level constants.Suggested fix
At the module level (e.g. near the type definition):
const EMPTY_COLUMN_FILTERS: never[] = []; const EMPTY_SORTING: never[] = [];Then reference them:
- defaultColumnFilters={[]} - defaultSorting={[]} + defaultColumnFilters={EMPTY_COLUMN_FILTERS} + defaultSorting={EMPTY_SORTING}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx around lines 463 - 468, The inline empty-array literals passed to DesignDataTable as defaultColumnFilters and defaultSorting create new references each render and can cause unnecessary recomputations; fix by hoisting them to module-level constants (e.g., EMPTY_COLUMN_FILTERS and EMPTY_SORTING) and then passing those constants to the DesignDataTable props defaultColumnFilters and defaultSorting instead of [] so the same reference is reused across renders.
398-447: Column definitions and row data are re-created on every render — consider memoizing.
columnsandendpointRowsare both defined inline in theelsebranch withoutuseMemo, so they produce new references on each render. This can causeDesignDataTable(and its internaluseReactTable) to reset or re-render unnecessarily.Additionally, Line 445 (
description: endpoint.description) is redundant after the...endpointspread.Suggested refactor
Move the column/row definitions above the conditional, wrapping them with
useMemo:function Endpoints(props: { updateFn: () => void, onTestRequested: (endpoint: Endpoint) => void }) { const endpoints = getSvixResult(useEndpoints({ limit: 100 })); + const columns: ColumnDef<Endpoint>[] = useMemo(() => [ + { + accessorKey: "url", + header: "Endpoint URL", + cell: ({ row }) => ( + <span className="text-sm font-medium text-foreground">{row.original.url}</span> + ), + }, + { + accessorKey: "description", + header: "Description", + cell: ({ row }) => ( + row.original.description ? ( + <span className="text-sm text-foreground/80">{row.original.description}</span> + ) : ( + <span className="text-sm text-muted-foreground">-</span> + ) + ), + }, + { + id: "status", + header: "Status", + cell: ({ row }) => ( + <DesignBadge + label={row.original.disabled ? "Disabled" : "Active"} + color={row.original.disabled ? "red" : "green"} + size="sm" + /> + ), + }, + { + id: "actions", + header: "", + cell: ({ row }) => ( + <div className="flex justify-end"> + <ActionMenu + endpoint={row.original} + updateFn={props.updateFn} + onTestEndpoint={props.onTestRequested} + /> + </div> + ), + }, + ], [props.updateFn, props.onTestRequested]); + + const endpointRows: Endpoint[] = useMemo(() => + endpoints.loaded + ? endpoints.data.map((endpoint) => ({ + ...endpoint, + disabled: endpoint.disabled ?? false, + })) + : [], + [endpoints.loaded, endpoints.data] + ); + if (!endpoints.loaded) { return endpoints.rendered; - } else { - const columns: ColumnDef<Endpoint>[] = [ ... ]; - const endpointRows: Endpoint[] = endpoints.data.map((endpoint) => ({ - ...endpoint, - description: endpoint.description, - disabled: endpoint.disabled ?? false, - })); - return ( ... ); + } + + return ( + <DesignCard ...> + <DesignDataTable + data={endpointRows} + columns={columns} + defaultColumnFilters={[]} + defaultSorting={[]} + /> + </DesignCard> + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx around lines 398 - 447, The columns and endpointRows are recreated on every render which causes unnecessary re-renders of DesignDataTable/useReactTable; wrap the columns array and the endpointRows mapping in React.useMemo to memoize them (e.g., const columns = useMemo(() => [...], [props.updateFn, props.onTestRequested]) and const endpointRows = useMemo(() => endpoints.data.map(e => ({ ...e, disabled: e.disabled ?? false })), [endpoints.data]) ), remove the redundant description: endpoint.description after the spread, and ensure dependencies include any props or values referenced by the memoized callbacks (like props.updateFn, props.onTestRequested and endpoints.data).apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx (4)
171-174:||used instead of explicit nullish check fordescription.
project.description || undefinedtreats empty strings as falsy and converts them toundefined. If this is intentional (empty string = no description), a comment would help clarify the intent. Otherwise,project.description ?? undefinedis the guideline-preferred pattern.As per coding guidelines, "Prefer explicit null/undefinedness checks over boolean checks, eg.
foo == nullinstead of!foo."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx around lines 171 - 174, The use of a boolean-falsy fallback incorrectly converts empty strings to undefined in projectDetailsDefaultValues; update the expression to use a nullish check so only null/undefined become undefined (replace project.description || undefined with a nullish pattern such as project.description ?? undefined), or if empty-string-to-undefined is intentional, add a clarifying comment next to projectDetailsDefaultValues explaining that empty strings should be treated as missing descriptions.
309-327: Dark-mode logo callbacks are inline while their light-mode counterparts are memoized.
handleLogoChangeandhandleFullLogoChange(lines 148-154) are extracted asuseCallbackmemos, but the dark-mode variants on lines 312-314 and 322-324 are defined inline. This inconsistency means the dark-modeLogoUploadcomponents receive a new function reference every render. Consider extracting them for consistency.♻️ Extract memoized callbacks
+ const handleLogoDarkModeChange = useCallback(async (logoDarkModeUrl: string | null) => { + await project.update({ logoDarkModeUrl }); + }, [project]); + + const handleLogoFullDarkModeChange = useCallback(async (logoFullDarkModeUrl: string | null) => { + await project.update({ logoFullDarkModeUrl }); + }, [project]);Then use
onValueChange={handleLogoDarkModeChange}andonValueChange={handleLogoFullDarkModeChange}in the JSX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx around lines 309 - 327, The dark-mode LogoUpload callbacks are defined inline causing new function refs each render; extract them as memoized callbacks like the existing handleLogoChange and handleFullLogoChange by creating useCallback hooks (e.g., handleLogoDarkModeChange and handleLogoFullDarkModeChange) that call project.update({ logoDarkModeUrl }) and project.update({ logoFullDarkModeUrl }) respectively, then pass those handlers as onValueChange to the dark-mode LogoUpload components so the props remain stable across renders.
88-101: URL construction uses raw string interpolation — guideline requiresencodeURIComponentorurlString.
project.idis interpolated directly into the URL template. While project IDs are likely safe identifiers, the coding guideline explicitly requires usingurlStringtagged templates orencodeURIComponent()for URL construction.♻️ Suggested fix
const jwksUrl = useMemo( - () => `${baseApiUrl}/api/v1/projects/${project.id}/.well-known/jwks.json`, + () => `${baseApiUrl}/api/v1/projects/${encodeURIComponent(project.id)}/.well-known/jwks.json`, [baseApiUrl, project.id] );Or use
urlStringif it's available in the project.As per coding guidelines, "Use urlString`` or encodeURIComponent() instead of normal string interpolation for URLs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx around lines 88 - 101, The three URL builders jwksUrl, restrictedJwksUrl and allJwksUrl currently interpolate project.id directly; update them to encode the project id (or use the project's urlString tagged template if available) so unsafe characters are escaped: replace `${project.id}` with an encoded value via encodeURIComponent(project.id) or use urlString`...${project.id}...` and keep the same dependency arrays ([baseApiUrl, project.id] and [jwksUrl]) so the useMemo hooks still update correctly.
30-30: Multipleanytypes without explanatory comments.
member: any(line 30)team: any(line 162)values: any(line 167)Per guidelines, the
anytype should be avoided, and when necessary a comment should explain why. Forvalues,yup.InferType<typeof projectInformationSchema>would be more precise and provide type safety on theproject.update(values)call. Formemberandteam, consider using the actual SDK types.♻️ Example: type-safe submit handler
- const handleProjectDetailsSubmit = useCallback(async (values: any) => { + const handleProjectDetailsSubmit = useCallback(async (values: yup.InferType<typeof projectInformationSchema>) => { await project.update(values); }, [project]);As per coding guidelines, "Try to avoid the
anytype. When necessary, leave a comment explaining why you're using it and why the type system fails there."Also applies to: 162-163, 167-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx at line 30, The code uses several any types (member in TeamMemberItem, team, and values) which reduces type safety; replace member and team with the correct SDK types from your project/team SDK (e.g., TeamMember or User types) by updating the TeamMemberItem prop signature and the team variable's type, and change values to a precise type using yup.InferType<typeof projectInformationSchema> so the call to project.update(values) is type-safe; if an SDK type is missing, add a short comment explaining why any is used and reference the intended type.apps/dashboard/src/components/smart-form.tsx (2)
39-52: Dependency array includesprops— callback recreated every render.
propsas a whole is a new object reference every render, so including it in theuseCallbackdeps makes the memoization effectively useless. Consider destructuring the specific props used (props.onChangeIsSubmitting,props.onSubmit) and listing them individually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/smart-form.tsx` around lines 39 - 52, handleSubmit is being recreated each render because the entire props object is in the useCallback dependency array; instead, destructure the specific callbacks you use (onChangeIsSubmitting and onSubmit) from props and reference those individual variables in the dependency list along with form and resolvedDefaultValues. Update the component to pull const { onChangeIsSubmitting, onSubmit } = props (or function params) and replace props.onChangeIsSubmitting/props.onSubmit calls inside handleSubmit, then change the useCallback deps to [onChangeIsSubmitting, onSubmit, form, resolvedDefaultValues] to restore proper memoization.
54-61:resolvedDefaultValuesis a new object reference every render, causing the effect to fire unnecessarily.
resolvedDefaultValues(line 32) is recomputed as a new object each render, so listing it in the dependency array means this effect runs every render. The guard condition (currentOpen && !prevOpen.current) prevents spurious resets, so this is not a correctness bug, but it's wasteful. Memoizing or stabilizing the defaults would make the intent clearer.♻️ Suggested stabilization
Wrap
resolvedDefaultValuesinuseMemoso the effect (andhandleSubmitcallback) only re-run when the values actually change:- const resolvedDefaultValues = props.defaultValues ?? props.formSchema.getDefault(); + const resolvedDefaultValues = useMemo( + () => props.defaultValues ?? props.formSchema.getDefault(), + [props.defaultValues, props.formSchema] + );Note: this still won't deep-stabilize if
props.defaultValuesis a new object with the same content each render (the caller should memoize). But it avoids the unconditional recompute ofgetDefault()and signals intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/smart-form.tsx` around lines 54 - 61, The effect depends on resolvedDefaultValues which is recomputed each render; stabilize it by wrapping the resolvedDefaultValues computation (the variable assigned at line 32) in useMemo so it only changes when its true inputs change (e.g., props.defaultValues or getDefault inputs), and then keep that memoized resolvedDefaultValues in the useEffect dependency array that calls form.reset; also ensure the handleSubmit callback (which uses resolvedDefaultValues) uses the memoized value so both the effect and callback only re-run when the actual defaults change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx:
- Around line 346-351: The conditional rendering for the error alert currently
checks errorMessage truthiness which hides empty-string errors; update the JSX
condition to explicitly check for null/undefined (e.g., errorMessage != null or
errorMessage !== null && errorMessage !== undefined) so that when status ===
'error' and errorMessage is an empty string the DesignAlert (title "Unable to
send event", component DesignAlert) still renders using
result.error.errorMessage; adjust the expression near the existing status ===
'error' && errorMessage to status === 'error' && errorMessage != null.
---
Outside diff comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx:
- Around line 289-300: sendTestEvent can throw if stackAdminApp.sendTestWebhook
rejects; wrap the await call in a try/catch so any thrown error sets local UI
state (call setStatus('error') and setErrorMessage with the caught error message
or a fallback) and avoid leaving the status stuck at 'sending'. In the catch
block reference stackAdminApp.sendTestWebhook failure, call setStatus('error'),
setErrorMessage(error?.message ?? 'Unexpected error'), and return (or ensure
finally behavior) so the Cancel button and UI are restored.
- Line 381: The route is built with a plain template literal; import the
urlString template tag from `@stackframe/stack-shared/dist/utils/urls` and use it
when calling router.push so path segments are properly encoded. Replace
router.push(`/projects/${project.id}/webhooks/${props.endpoint.id}`) with
router.push(urlString`/projects/${project.id}/webhooks/${props.endpoint.id}`)
and add the corresponding import for urlString at the top of the module;
reference symbols: router.push, project.id, props.endpoint.id, and urlString.
---
Duplicate comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx:
- Around line 275-284: The default values are correctly wired:
projectDetailsDefaultValues (memo defined earlier) is being passed into
SmartFormDialog's defaultValues prop and pre-populates the form; no code change
required—approve the change as-is and ensure SmartFormDialog,
projectDetailsDefaultValues, and handleProjectDetailsSubmit remain unchanged.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx:
- Around line 417-427: Status column was updated to remove the hardcoded
"Active" label; ensure the column definition with id "status" uses
row.original.disabled to conditionally set DesignBadge's label ("Disabled" vs
"Active") and color ("red" vs "green"). Locate the column object (id: "status")
and confirm the cell renderer returns <DesignBadge label={row.original.disabled
? "Disabled" : "Active"} color={row.original.disabled ? "red" : "green"}
size="sm" /> so the UI reflects the disabled field correctly.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/project-settings/page-client.tsx:
- Around line 171-174: The use of a boolean-falsy fallback incorrectly converts
empty strings to undefined in projectDetailsDefaultValues; update the expression
to use a nullish check so only null/undefined become undefined (replace
project.description || undefined with a nullish pattern such as
project.description ?? undefined), or if empty-string-to-undefined is
intentional, add a clarifying comment next to projectDetailsDefaultValues
explaining that empty strings should be treated as missing descriptions.
- Around line 309-327: The dark-mode LogoUpload callbacks are defined inline
causing new function refs each render; extract them as memoized callbacks like
the existing handleLogoChange and handleFullLogoChange by creating useCallback
hooks (e.g., handleLogoDarkModeChange and handleLogoFullDarkModeChange) that
call project.update({ logoDarkModeUrl }) and project.update({
logoFullDarkModeUrl }) respectively, then pass those handlers as onValueChange
to the dark-mode LogoUpload components so the props remain stable across
renders.
- Around line 88-101: The three URL builders jwksUrl, restrictedJwksUrl and
allJwksUrl currently interpolate project.id directly; update them to encode the
project id (or use the project's urlString tagged template if available) so
unsafe characters are escaped: replace `${project.id}` with an encoded value via
encodeURIComponent(project.id) or use urlString`...${project.id}...` and keep
the same dependency arrays ([baseApiUrl, project.id] and [jwksUrl]) so the
useMemo hooks still update correctly.
- Line 30: The code uses several any types (member in TeamMemberItem, team, and
values) which reduces type safety; replace member and team with the correct SDK
types from your project/team SDK (e.g., TeamMember or User types) by updating
the TeamMemberItem prop signature and the team variable's type, and change
values to a precise type using yup.InferType<typeof projectInformationSchema> so
the call to project.update(values) is type-safe; if an SDK type is missing, add
a short comment explaining why any is used and reference the intended type.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/webhooks/page-client.tsx:
- Around line 463-468: The inline empty-array literals passed to DesignDataTable
as defaultColumnFilters and defaultSorting create new references each render and
can cause unnecessary recomputations; fix by hoisting them to module-level
constants (e.g., EMPTY_COLUMN_FILTERS and EMPTY_SORTING) and then passing those
constants to the DesignDataTable props defaultColumnFilters and defaultSorting
instead of [] so the same reference is reused across renders.
- Around line 398-447: The columns and endpointRows are recreated on every
render which causes unnecessary re-renders of DesignDataTable/useReactTable;
wrap the columns array and the endpointRows mapping in React.useMemo to memoize
them (e.g., const columns = useMemo(() => [...], [props.updateFn,
props.onTestRequested]) and const endpointRows = useMemo(() =>
endpoints.data.map(e => ({ ...e, disabled: e.disabled ?? false })),
[endpoints.data]) ), remove the redundant description: endpoint.description
after the spread, and ensure dependencies include any props or values referenced
by the memoized callbacks (like props.updateFn, props.onTestRequested and
endpoints.data).
In `@apps/dashboard/src/components/smart-form.tsx`:
- Around line 39-52: handleSubmit is being recreated each render because the
entire props object is in the useCallback dependency array; instead, destructure
the specific callbacks you use (onChangeIsSubmitting and onSubmit) from props
and reference those individual variables in the dependency list along with form
and resolvedDefaultValues. Update the component to pull const {
onChangeIsSubmitting, onSubmit } = props (or function params) and replace
props.onChangeIsSubmitting/props.onSubmit calls inside handleSubmit, then change
the useCallback deps to [onChangeIsSubmitting, onSubmit, form,
resolvedDefaultValues] to restore proper memoization.
- Around line 54-61: The effect depends on resolvedDefaultValues which is
recomputed each render; stabilize it by wrapping the resolvedDefaultValues
computation (the variable assigned at line 32) in useMemo so it only changes
when its true inputs change (e.g., props.defaultValues or getDefault inputs),
and then keep that memoized resolvedDefaultValues in the useEffect dependency
array that calls form.reset; also ensure the handleSubmit callback (which uses
resolvedDefaultValues) uses the memoized value so both the effect and callback
only re-run when the actual defaults change.
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Full Playground with multi-component live previews and generated TSX snippets. * New reusable Design data table and richer editable-grid interfaces. * **Refactor** * Replaced legacy UI primitives across many pages with unified Design components (cards, buttons, alerts, badges, tabs). * **Improvements** * PageLayout supports optional content overflow. * Enhanced inline editing, copy-to-clipboard, async toggles with confirmation dialogs, dialog/form default value handling, and consistent alerts/UX. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Konstantin Wohlwend <n2d4xc@gmail.com> Co-authored-by: nams1570 <amanganapathy@gmail.com>
Summary by CodeRabbit
New Features
Refactor
Improvements