feat: add secrete auth on mobile and index performance improvements - #60
feat: add secrete auth on mobile and index performance improvements#60remcostoeten wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Sorry @remcostoeten, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR introduces a comprehensive component studio and animation editor system with extensive UI components, API routes, and refactored data fetching. It removes configuration test outputs, adds Radix UI dependencies with a bun-based build workflow, replaces the playground with an interactive multi-level navigation system, and refactors project showcase to use client-side data fetching via new API routes. Changes
Sequence DiagramsequenceDiagram
participant User
participant PlaygroundLayout as PlaygroundLayout<br/>(State Manager)
participant PropEditor as PropEditor
participant AnimationStudio as AnimationStudio
participant ComponentPreview as ComponentPreview
participant CodeExportPanel as CodeExportPanel
User->>PlaygroundLayout: Select component/update props
PlaygroundLayout->>PropEditor: Pass schema, values
PropEditor->>PropEditor: Render prop controls
User->>PropEditor: Edit prop value
PropEditor->>PlaygroundLayout: onChange(name, value)
PlaygroundLayout->>PlaygroundLayout: Update state.props
PlaygroundLayout->>ComponentPreview: Pass merged props
ComponentPreview->>ComponentPreview: Render live preview
ComponentPreview->>User: Display updated component
User->>PlaygroundLayout: Configure animation
PlaygroundLayout->>AnimationStudio: Pass animation config
AnimationStudio->>AnimationStudio: Handle keyframes, timing
User->>AnimationStudio: Adjust bezier/duration
AnimationStudio->>PlaygroundLayout: Dispatch animation updates
PlaygroundLayout->>PlaygroundLayout: Generate animation CSS
PlaygroundLayout->>ComponentPreview: Pass animationKeyframesCSS
ComponentPreview->>User: Preview animated component
User->>CodeExportPanel: Request code
CodeExportPanel->>CodeExportPanel: Generate JSX via generateComponentJsx
CodeExportPanel->>User: Display copy-ready code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/auth/vim-auth-provider.tsx (1)
67-79:⚠️ Potential issue | 🟡 MinorFire-and-forget
signOut()with no error handling.If
signOut()rejects (e.g., network error), the promise is silently swallowed. Consider awaiting or catching to avoid an unhandled rejection.Proposed fix
useEffect(() => { if (session?.user) { const isAllowed = session.user.name?.toLowerCase() === ALLOWED_GITHUB_USERNAME if (!isAllowed) { console.warn( 'Unauthorized user attempted login, signing out...' ) - signOut() + signOut().catch((err) => + console.error('Failed to sign out unauthorized user:', err) + ) } } }, [session])
🤖 Fix all issues with AI agents
In `@src/app/api/projects/git-metrics/route.ts`:
- Around line 24-32: Replace the unbounded Promise.all over dbProjects that
calls fetchGitMetrics with a bounded-concurrency or batched approach (e.g., use
p-limit or process dbProjects in chunks) to avoid hitting GitHub rate limits;
ensure each call to fetchGitMetrics is wrapped in a try/catch so failures or
403s for a single project do not reject the whole operation, and still populate
metricsByProject[project.title] only when metrics is successfully returned.
In `@src/app/fonts.ts`:
- Line 6: The constant ENABLE_PIXEL_FONT is hardcoded true; change it to default
to false and drive it from an environment/build flag instead so it can't ship
enabled accidentally—replace the literal export of ENABLE_PIXEL_FONT with logic
that reads a public env var (e.g., NEXT_PUBLIC_ENABLE_PIXEL_FONT or similar) and
converts it to a boolean, falling back to false when unset or invalid; ensure
the flag is exposed to the client runtime (or build-time) per your framework
conventions and update any related docs/tests that assume the pixel font is
enabled.
In `@src/components/component-studio/playground/component-preview.tsx`:
- Around line 41-43: The component currently injects user-edited CSS via
animationKeyframesCSS into a <style> tag using dangerouslySetInnerHTML; replace
this by sanitizing and/or programmatically constructing the style content:
validate keyframe inputs in the keyframe editor pipeline (allowlist permitted
CSS property names and patterns and reject or escape unsafe values like
urls/data URIs), or build the style element via DOM APIs inside ComponentPreview
(or the function that produces animationKeyframesCSS) and set textContent
instead of using dangerouslySetInnerHTML; ensure you reference and sanitize
values that feed animationKeyframesCSS and centralize the validation logic so
only safe CSS is emitted.
In `@src/components/component-studio/playground/playground-layout.tsx`:
- Around line 196-261: The computed animation name stored in animNameRef (set in
the useEffect that depends on animKeyframeHash) is stale when read inside the
animationCSS useMemo because useEffect runs after render; replace this with
either (A) derive the name inside the same useMemo that builds animationCSS
(remove animNameRef and the useEffect) so the name is fresh whenever
animKeyframeHash changes, or (B) switch animNameRef to a state value (e.g.,
animName via useState and setAnimName in an effect) and include that state
(animName) in the animationCSS useMemo dependencies so the new name triggers a
re-render and the fresh name is used when building `@keyframes`; update references
to animNameRef.current to use the new local name/state.
In `@src/components/layout/footer.tsx`:
- Around line 60-76: The click-count timeout in handleSecretAuth uses
independent setTimeouts that are never cleared, causing stale timers to reset
clickCount mid-sequence; fix by adding a ref (e.g., timeoutRef via useRef<number
| null>) to store the active timeout id, call clearTimeout(timeoutRef.current)
at the start of handleSecretAuth before creating a new setTimeout, assign the
new timeout id to timeoutRef.current, and reset timeoutRef.current to null when
the timeout fires or when you trigger openAuthModal; also add a useEffect
cleanup to clearTimeout(timeoutRef.current) on unmount.
In `@src/components/projects/components/project-showcase.tsx`:
- Around line 36-44: Destructure isError, error and refetch from the useQuery
call in ProjectShowcase (alongside data and isLoading) and change the render
guard so the skeleton shows only while isLoading; when isError or data is
undefined render an error fallback UI that displays error?.message (and a retry
button that calls refetch) instead of the skeleton. Specifically update the
useQuery call that uses fetchProjectShowcaseData to include
isError/error/refetch and replace the current "isLoading || !data" check with an
ordered conditional: if (isLoading) -> skeleton, else if (isError || !data) ->
error fallback (show error message and retry), else -> render the normal project
grid.
In `@src/components/seo/web-vitals-reporter.tsx`:
- Around line 29-66: The component WebVitalsReporter currently returns early
before calling useEffect which violates the Rules of Hooks; remove the top-level
NODE_ENV early return so useEffect is always invoked, then move the
development-mode guard inside the effect (at the start of the effect callback)
to bail out immediately if process.env.NODE_ENV !== 'development'; keep the
existing setup async function, cancelled flag, logMetric, and cleanup logic
(returning the cleanup that sets cancelled) and maintain the empty dependency
array so onLCP/onFCP/onCLS/onTTFB are only registered in development builds.
In `@src/components/ui/hero-pill.tsx`:
- Around line 46-51: The effectiveVariant computation incorrectly falls back to
the original variant (which may be "ghost") when the ghost effect is inactive;
update the logic in effectiveVariant (and the PillVariant usage) so that when
variant === "ghost" but ghostBehavior is "idle" and isIdle is false, or when
ghostBehavior is "never", it returns a non-ghost variant (e.g., "default")
instead of variant; implement this by changing the ternary to explicitly return
"default" (or use a new prop activeVariant to allow callers to override the
non-ghost appearance) whenever the ghost behavior is not active, and update any
call sites that may rely on the old fallback.
🟡 Minor comments (16)
src/app/fonts.ts-14-19 (1)
14-19:⚠️ Potential issue | 🟡 MinorGeist CSS variables are unavailable when pixel font is enabled.
When
ENABLE_PIXEL_FONTistrue, neitherGeistSans.variablenorGeistMono.variableis included in the output. Any component or Tailwind utility referencing--font-geist-sansor--font-geist-monowill silently fall back to the browser default. If the intent is a full pixel-font takeover this is fine, but if some components (e.g., code blocks) should still use Geist Mono, you'd want to preserve those variables:if (ENABLE_PIXEL_FONT) { - return `${pixelFont.variable} ${pixelFont.className}` + return `${GeistSans.variable} ${GeistMono.variable} ${pixelFont.variable} ${pixelFont.className}` }src/components/ui/gooey-toggle.tsx-74-82 (1)
74-82:⚠️ Potential issue | 🟡 Minor
{...props}spread could overridetype="checkbox".Since
typeis not destructured from props, a consumer passingtype="radio"(ortype="text") would silently override the checkbox behavior. Move the spread before the explicit attributes, or destructure and discardtype.Proposed fix
<input - type="checkbox" - ref={ref} - checked={isChecked} - onChange={handleChange} - data-checked={isChecked} - className={inputStyles} {...props} + type="checkbox" + ref={ref} + checked={isChecked} + onChange={handleChange} + data-checked={isChecked} + className={inputStyles} />src/app/api/projects/showcase/route.ts-19-31 (1)
19-31:⚠️ Potential issue | 🟡 MinorEmpty
githubURL will produce a broken link in the UI.Line 25: when
gitUrlisnull,githubbecomes''. BothProjectRowandProjectCardunconditionally render an<a href={project.github}>GitHub link. An emptyhrefnavigates to the current page, which is confusing.Consider either filtering out projects without a
gitUrlfrom the response, or setting a sentinel that the UI can check to conditionally hide the link.One possible approach — omit the link-target when missing
- github: dbProject.gitUrl ?? '', + github: dbProject.gitUrl ?? undefined!,A cleaner fix would be to make
githuboptional inIProjectand guard the<a>in the card/row components.src/hooks/use-idle-detection.ts-5-9 (1)
5-9:⚠️ Potential issue | 🟡 Minor
elementRefparameter is accepted but never used.The effect attaches listeners to
documentglobally, yetelementRefis a required parameter and sits in the dependency array (Line 37). This is misleading — callers must create and pass a ref that has zero effect on behavior. Either scope the listeners toelementRef.current(which was likely the intent) or remove the parameter.Option A: scope listeners to the element
useEffect(() => { if (!enabled) { setIsIdle(false) return } + const el = elementRef.current + if (!el) return let timeoutId: ReturnType<typeof setTimeout> const resetTimer = () => { setIsIdle(false) clearTimeout(timeoutId) timeoutId = setTimeout(() => setIsIdle(true), idleMs) } const events = ["mousemove", "mousedown", "scroll", "touchstart", "keydown"] - events.forEach((e) => - document.addEventListener(e, resetTimer, { passive: true }) - ) + for (const e of events) { + el.addEventListener(e, resetTimer, { passive: true }) + } timeoutId = setTimeout(() => setIsIdle(true), idleMs) return () => { clearTimeout(timeoutId) - events.forEach((e) => document.removeEventListener(e, resetTimer)) + for (const e of events) { + el.removeEventListener(e, resetTimer) + } } }, [elementRef, idleMs, enabled])Option B: remove the unused parameter (if global detection is intentional)
-export function useIdleDetection( - elementRef: RefObject<HTMLElement | null>, - idleMs: number, - enabled: boolean -): boolean { +export function useIdleDetection( + idleMs: number, + enabled: boolean +): boolean {This would also require updating callers (e.g.,
hero-pill.tsx).Also applies to: 37-37
src/hooks/use-idle-detection.ts-26-29 (1)
26-29:⚠️ Potential issue | 🟡 MinorFix lint:
forEachcallbacks should not return a value.The arrow expressions implicitly return the result of
addEventListener/removeEventListener. Use block bodies to satisfy the Biome ruleuseIterableCallbackReturn.Proposed fix
- events.forEach((e) => - document.addEventListener(e, resetTimer, { passive: true }) - ) + events.forEach((e) => { + document.addEventListener(e, resetTimer, { passive: true }) + })- events.forEach((e) => document.removeEventListener(e, resetTimer)) + events.forEach((e) => { document.removeEventListener(e, resetTimer) })Or use
for...ofloops as shown in Option A above, which avoids the issue entirely.Also applies to: 33-36
src/components/component-studio/lib/jsx-utils.ts-6-8 (1)
6-8:⚠️ Potential issue | 🟡 MinorString values are not escaped — embedded quotes will produce invalid JSX.
If a string prop value contains double quotes (e.g.,
hello "world"), the output becomes"hello "world"", which is malformed JSX. Escape special characters before interpolating.Proposed fix
if (typeof value === "string") { - return `"${value}"` + return `"${value.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"` }src/components/component-studio/playground/animation-studio/playback-controls.tsx-220-247 (1)
220-247:⚠️ Potential issue | 🟡 MinorSpeed input and slider have mismatched max values (1000 vs 500).
The numeric input (Line 225) accepts speeds up to
1000while theSlider(Line 239) caps atmax={500}. If a user types 750 into the input, the slider handle overflows or clips, and dragging the slider afterward would snap the value down. Align the ranges or document the intentional discrepancy.Proposed fix — align both to the same max
onChange={(e) => { const v = parseInt(e.target.value, 10) - if (!isNaN(v) && v >= 1 && v <= 1000) onSpeedChange(v) + if (!isNaN(v) && v >= 1 && v <= 500) onSpeedChange(v) }} min={1} - max={1000} + max={500}Or raise the slider max to 1000 and update the scale labels accordingly.
src/components/component-studio/playground/playground-toolbar.tsx-16-19 (1)
16-19:⚠️ Potential issue | 🟡 MinorUnhandled promise rejection from clipboard API.
navigator.clipboard.writeTextreturns a Promise that can reject (e.g., non-secure context, permission denied, or unsupported browser). This will produce an unhandled promise rejection at runtime.Proposed fix
const copyUrl = () => { - navigator.clipboard.writeText(window.location.href) - toast("URL copied to clipboard") + navigator.clipboard.writeText(window.location.href).then( + () => toast("URL copied to clipboard"), + () => toast.error("Failed to copy URL") + ) }src/components/component-studio/lib/jsx-utils.ts-79-80 (1)
79-80:⚠️ Potential issue | 🟡 Minor
startsWithcheck can falsely match prop names that share a prefix.If the schema has props like
textandtextColor,p.trim().startsWith("text")matches both, potentially preventingtextfrom being added as a required prop.Proposed fix — match the exact prop name followed by a delimiter
- if (!propEntries.some((p) => p.trim().startsWith(propSchema.name))) { + if (!propEntries.some((p) => { + const trimmed = p.trim() + return trimmed === propSchema.name || trimmed.startsWith(propSchema.name + "=") || trimmed.startsWith(propSchema.name + "{") + })) {src/components/providers/posthog-provider.tsx-33-42 (1)
33-42:⚠️ Potential issue | 🟡 Minor
defaults: '2025-11-30'is not a valid value for thedefaultsconfig option.The
defaultsproperty is a recognized PostHog configuration option used to enable/disable breaking change defaults. However, the only valid values are'2025-05-24'(enable updated default behaviors) or'unset'(use legacy defaults). Using an invalid value like'2025-11-30'will be silently ignored or cause a TypeScript type error. Change this to either'2025-05-24'or'unset'.src/components/component-studio/playground/code-export-panel.tsx-88-106 (1)
88-106:⚠️ Potential issue | 🟡 MinorHardcoded import path in
baseCodemay not match actual component locations.Line 93 and 105 generate an import statement using
@/components/ui/${registration.slug}, but the component's actual file path may differ from its slug. For example, a component with slug"gooey-toggle"would produce@/components/ui/gooey-toggle, which may or may not exist. Consider adding an optionalimportPathfield toComponentRegistrationto make this accurate.src/components/component-studio/playground/code-export-panel.tsx-3-4 (1)
3-4:⚠️ Potential issue | 🟡 MinorUnused import:
Clipboard.
Clipboardis imported fromlucide-reacton line 4 but is never used in this file. OnlyCheckandCopyare referenced.Proposed fix
-import { Check, Copy, Clipboard } from "lucide-react" +import { Check, Copy } from "lucide-react"package.json-4-5 (1)
4-5:⚠️ Potential issue | 🟡 MinorInconsistency:
build:fastuses plainnext buildinstead ofnext --bun build.The
buildscript usesnext --bun build, butbuild:fastuses plainnext buildwithout the--bunflag. Whilebuild:fastappears intentional (it setsFAST_BUILD=trueto disable TypeScript error checking innext.config.mjs), ensure that production deployments use thebuildscript, notbuild:fast, to maintain consistent Bun runtime usage. The project is already Bun-configured (bun.lock,vercel.jsonwith"bunVersion": "1.x"), so the main concern is preventing accidental use ofbuild:fastin production.src/components/component-studio/playground/animation-studio/animation-export.tsx-77-80 (1)
77-80:⚠️ Potential issue | 🟡 Minor
navigator.clipboard.writeTextis not awaited and has no error handling.If clipboard access is denied (e.g., iframe restrictions, no user gesture, non-secure context), this will throw an unhandled promise rejection, and the toast will display "Copied" even though the copy failed.
🛡️ Proposed fix
- const copy = (text: string) => { - navigator.clipboard.writeText(text) - toast("Copied to clipboard") + const copy = async (text: string) => { + try { + await navigator.clipboard.writeText(text) + toast("Copied to clipboard") + } catch { + toast.error("Failed to copy") + } }src/components/ui/pill-showcase.tsx-222-253 (1)
222-253:⚠️ Potential issue | 🟡 MinorClickable
<div>lacks keyboard accessibility.These grid items act as interactive buttons but are not focusable or operable via keyboard. Screen readers and keyboard-only users cannot activate them.
Add
role="button",tabIndex={0}, and anonKeyDownhandler for Enter/Space:♿ Proposed fix
<div key={variant.id} className="group relative flex aspect-[4/3] flex-col items-center justify-center gap-3 rounded-xl border border-border/40 bg-muted/10 p-4 transition-all hover:border-border/80 hover:bg-muted/30 cursor-pointer" onClick={() => handleCopy(variant)} + onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); handleCopy(variant) } }} + role="button" + tabIndex={0} + aria-label={`Copy ${variant.label} variant code`} >src/components/component-studio/playground/animation-studio/animation-studio.tsx-178-181 (1)
178-181:⚠️ Potential issue | 🟡 MinorDuration validation doesn't match the HTML
min/maxconstraints.The JS handler accepts any value
> 0, but the<Input>declaresmin={10}andmax={30000}. A user can type a value like5or99999and the dispatch will accept it, bypassing the intended constraints.🛡️ Proposed fix to align JS validation with HTML constraints
onChange={(e) => { const v = parseInt(e.target.value, 10) - if (!isNaN(v) && v > 0) + if (!isNaN(v) && v >= 10 && v <= 30000) dispatch({ type: "SET_DURATION", value: v }) }}
🧹 Nitpick comments (29)
src/components/providers/dev-widget-wrapper.tsx (1)
3-4: Inconsistent import path style.Line 3 uses a relative path (
../../../tools/dev-menu) while Line 4 uses the@/alias. Prefer the alias for consistency and readability.-import { DevWidget } from '../../../tools/dev-menu' +import { DevWidget } from '@/tools/dev-menu'src/components/ui/gooey-toggle.tsx (2)
104-106: Conditional render prevents the drop circle from animating in.Because the circle is mounted/unmounted via
{isChecked && ...}, the CSStransition-transformondropCircleStyleshas no effect on enter — the element simply pops in. If a smooth entrance is intended, consider always rendering the circle and toggling its scale/opacity instead.Example: always-mounted with animated scale
- {isChecked && ( - <circle className={dropCircleStyles} cx="35" cy="-1" r="2.5" /> - )} + <circle + className={dropCircleStyles} + cx="35" + cy="-1" + r="2.5" + style={{ + transform: `scale(${isChecked ? 1 : 0})`, + transformOrigin: "35px -1px", + }} + />
112-129: Hardcodedid="gooey-filter"will cause duplicate-ID issues ifGooeyFilteris rendered more than once.If multiple layout segments or component trees mount
GooeyFilter, the duplicate SVG filter ID can cause unpredictable rendering. Consider documenting that it must be a singleton (e.g., placed in the root layout), or accept an optionalidprop to disambiguate.src/components/layout/footer.tsx (2)
68-71: Toast reveals the secret auth mechanism.Showing
'Secret auth triggered!'via a toast notification somewhat defeats the purpose of a secret auth trigger. Consider removing the toast or making it more subtle (e.g., only display it in dev mode).
160-165:active:scale-95hints at interactivity on a "hidden" trigger.The press animation is a visual clue that the copyright text is interactive. If secrecy is a goal, consider removing
active:scale-95.src/components/projects/components/project-preview.tsx (1)
70-70:loadingternary is dead code —isVisibleis alwaystruehere.Line 20 returns
nullwhen!isVisible, so any code past that guard can only execute whenisVisible === true. The ternaryisVisible ? 'eager' : 'lazy'will always evaluate to'eager'. Either simplify toloading="eager"or, if lazy loading was intentional for some future path, move the early return.Suggested simplification
<iframe src={preview.embedUrl || preview.url} title={name} - loading={isVisible ? 'eager' : 'lazy'} + loading="eager" sandbox="allow-scripts allow-same-origin"src/components/projects/components/project-card.tsx (1)
90-94:forceShowPreviewsync effect is clean, but consider the interaction with local toggle.When
forceShowPreviewflips fromtrue→undefined, the effect on line 91 won't fire (conditionforceShowPreview !== undefinedis false), leavingshowPreviewin whatever state it was forced to. This seems intentional (user's local toggle takes over), but worth confirming the expected UX.src/components/projects/components/project-showcase.tsx (1)
75-91: Duplicated merge logic forfeaturedandother— consider extracting a helper.The mapping logic on lines 75–82 and 84–91 is identical. A small helper would reduce duplication:
Suggested refactor
+const mergeGitMetrics = (projects: IProject[], metrics?: ProjectGitMetricsMap) => + projects.map(project => ({ + ...project, + git: metrics?.[project.name] ?? project.git + })) + -const featured = useMemo( - () => - (data?.featured ?? []).map(project => ({ - ...project, - git: gitMetrics?.[project.name] ?? project.git - })), - [data?.featured, gitMetrics] -) - -const other = useMemo( - () => - (data?.other ?? []).map(project => ({ - ...project, - git: gitMetrics?.[project.name] ?? project.git - })), - [data?.other, gitMetrics] -) +const featured = useMemo( + () => mergeGitMetrics(data?.featured ?? [], gitMetrics), + [data?.featured, gitMetrics] +) + +const other = useMemo( + () => mergeGitMetrics(data?.other ?? [], gitMetrics), + [data?.other, gitMetrics] +)src/components/projects/components/project-card-skeleton.tsx (1)
5-42:FeaturedCardSkeletoninproject-showcase-skeleton.tsxduplicates this component almost verbatim.Both
ProjectCardSkeleton(this file) and the internalFeaturedCardSkeleton(inproject-showcase-skeleton.tsx, lines 6–39) render the same skeleton markup with the samewithPreviewprop. Consider reusingProjectCardSkeletoninsideProjectShowcaseSkeletonto keep a single source of truth:// In project-showcase-skeleton.tsx import { ProjectCardSkeleton } from './project-card-skeleton' // Then replace FeaturedCardSkeleton usage: <ProjectCardSkeleton withPreview={i === 0} />src/app/(marketing)/page.tsx (2)
44-53: Redundant lazy-loading layers:nextDynamic+DeferredRender+useQueryskeleton.
ProjectShowcasegoes through three sequential loading phases, each showing a skeleton:
DeferredRender— shows<ProjectShowcaseSkeleton>until the section scrolls into viewnextDynamic({ ssr: false })— shows its ownloadingskeleton while the JS chunk downloadsuseQueryinsideProjectShowcase— shows yet another skeleton while fetching API dataSince
DeferredRenderalready defers rendering until the viewport is near, andProjectShowcaseisssr: false(so it won't block the page), wrapping it inDeferredRenderadds marginal value while creating a triple-skeleton waterfall. Consider either:
- Removing
DeferredRenderand relying onnextDynamic+ the internaluseQueryskeleton, or- Removing
nextDynamicand using onlyDeferredRenderwith a direct import (the component already handles its own loading).The same pattern applies to
ActivitySection(lines 92–97) andPlayground(lines 112–119).Also applies to: 99-110
6-6: Import path inconsistency: direct path vs. barrel export.
ProjectShowcaseSkeletonis imported from the deep component path here, but it's also exported from the barrelsrc/components/projects/index.ts(line 5). For consistency with howProjectShowcaseis imported (via barrel in the dynamic import on line 46–48), consider using:-import { ProjectShowcaseSkeleton } from '@/components/projects/components/project-showcase-skeleton' +import { ProjectShowcaseSkeleton } from '@/components/projects'src/components/projects/components/project-showcase-skeleton.tsx (1)
62-62: Magic number40duplicatesROW_HEIGHTfrom the client component.The value
40on this line mirrorsROW_HEIGHT = 40defined inproject-showcase-client.tsx(line 15). If the row height changes in one place but not the other, the skeleton and the real component will visually mismatch, causing a layout shift on load.Consider sharing a constant:
Suggested approach
// e.g., in a shared constants file or co-located export const ROW_HEIGHT = 40 // Then import in both project-showcase-client.tsx and project-showcase-skeleton.tsxsrc/components/ui/slider.tsx (1)
8-26: SingleThumblimits this to single-value sliders.Radix Slider supports multiple values (e.g., range sliders) by rendering a Thumb per value. This implementation hardcodes one Thumb, so passing
defaultValue={[25, 75]}would visually break. Fine if only single-value use is planned, but worth noting for future consumers.src/components/landing/playground.tsx (1)
1-1:"use client"may be unnecessary here.This component doesn't use any hooks, event handlers, or browser APIs directly. Child components like
HeroPillalready declare their own"use client"boundary. Removing the directive would let Next.js render this as a server component, keeping it out of the client bundle.src/components/component-studio/lib/jsx-utils.ts (1)
34-65: Duplicate iteration pattern for props and behaviors — consider extracting a helper.The prop-formatting logic in lines 35–49 and 52–65 is nearly identical. A small shared helper would reduce duplication and make future format changes less error-prone.
src/app/(marketing)/playground/page.tsx (1)
122-123:GooeyFilteris rendered for every category, not just the one that uses it.
<GooeyFilter />injects an SVG filter definition into the DOM for all category views. It's harmless (just an invisible SVG<defs>), but it's slightly cleaner to conditionally render it only when the category actually contains gooey-toggle components, or to move it into the component preview that needs it.src/components/component-studio/playground/code-export-panel.tsx (3)
53-55: Move PrismJS imports to the top of the file.Imports are conventionally placed at the top of the file. Having them between component definitions (after
CopyButton, beforeCodeBlock) is unconventional and can be confusing to readers.
57-75:dangerouslySetInnerHTMLwith PrismJS output — acceptable here, but note the static analysis flags.Static analysis tools flag line 71 for XSS risk. In this case,
Prism.highlight()processes internally-generated JSX strings (fromgenerateComponentJsx) and HTML-encodes the source text before wrapping tokens in<span>elements, so this is safe. The input is developer-controlled registry data, not arbitrary user input.If you want to eliminate the lint warnings entirely, consider a React-based syntax highlighter (e.g.,
react-syntax-highlighter, which is already in yourpackage.jsondependencies) instead of raw Prism +dangerouslySetInnerHTML.
18-51: DuplicateCopyButtonimplementation.There is an existing
CopyButtoncomponent atsrc/components/playground/copy-button.tsxwith very similar functionality. Consider reusing that component (or extracting a shared one) to avoid duplication.#!/bin/bash # Verify the existing CopyButton component fd "copy-button" --type f --exec cat {}src/components/component-studio/playground/prop-controls/icon-picker-control.tsx (1)
58-73: Icon buttons lack accessible labels for screen readers.The icon
<button>elements only have atitleattribute but no visible text oraria-label. Screen readers won't announce what each button represents.Suggested fix
<button key={key} onClick={() => { onChange(key) setOpen(false) }} className={cn( "group flex h-10 w-full items-center justify-center rounded-none border border-transparent transition-colors hover:bg-accent/40", value === key && "border-border/50 bg-accent/20" )} title={key} + aria-label={key} >src/hooks/use-animation-playback.ts (1)
49-108:progressin the dependency array causes the effect to re-run every frame.Since
onProgressChangeupdatesprogresson every animation tick, this effect tears down and re-creates the rAF loop each frame. It works but adds unnecessary overhead (effect cleanup, closure allocation, rAF cancellation + re-scheduling ~60 times/sec).A more efficient pattern: store
progress,currentRun, etc. in refs so thetickclosure always reads fresh values without requiring the effect to re-run. The effect would then only depend onisPlayingandisInDelayto start/stop the loop.Sketch of the ref-based approach
+ const progressRef = useRef(progress) + const currentRunRef = useRef(currentRun) + useEffect(() => { progressRef.current = progress }, [progress]) + useEffect(() => { currentRunRef.current = currentRun }, [currentRun]) useEffect(() => { if (!isPlaying || isInDelay) { cancelAnimationFrame(rafRef.current) return } lastTimeRef.current = performance.now() const tick = (now: number) => { const delta = now - lastTimeRef.current lastTimeRef.current = now - const increment = (delta / duration) * 100 * speedMultiplier - const next = progress + increment + const increment = (delta / duration) * 100 * speedMultiplier + const next = progressRef.current + increment // ... rest uses refs instead of closure-captured state rafRef.current = requestAnimationFrame(tick) } rafRef.current = requestAnimationFrame(tick) return () => cancelAnimationFrame(rafRef.current) - }, [isPlaying, isInDelay, duration, speedMultiplier, progress, currentRun, ...]) + }, [isPlaying, isInDelay, duration, speedMultiplier])src/components/component-studio/playground/animation-studio/bezier-editor.tsx (1)
40-76:fromSvgis recreated every render, breakinguseCallbackmemoization ofhandlePointerMove.
fromSvgis a plain inline function, so it's a new reference on each render. Since it appears in the dependency array ofhandlePointerMove(line 75), the callback is never actually memoized. WrapfromSvginuseCallback(it only depends onsize).Suggested fix
- const fromSvg = (sx: number, sy: number): [number, number] => [ - Math.round((sx / size) * 100) / 100, - Math.round(((1 - sy / size)) * 100) / 100, - ] + const fromSvg = useCallback( + (sx: number, sy: number): [number, number] => [ + Math.round((sx / size) * 100) / 100, + Math.round(((1 - sy / size)) * 100) / 100, + ], + [size] + )src/components/component-studio/playground/component-preview.tsx (1)
49-49: Redundant type guard.
variantPropis already found withp.type === "enum"on line 35, making the secondvariantProp.type === "enum"check on line 49 unnecessary.Suggested fix
- {variantProp && variantProp.type === "enum" && ( + {variantProp && (src/components/component-studio/playground/animation-studio/timeline.tsx (2)
87-105: Wheel handler re-attaches on everyprogresschange (~60fps during playback).
progressis in the dependency array, so during active playback the event listener is removed and re-added every frame. Use a ref to hold the current progress value so the effect only needs to depend onzoom,onZoomChange, andonProgressChange.Suggested fix
+ const progressRef = useRef(progress) + useEffect(() => { progressRef.current = progress }, [progress]) + const zoomRef = useRef(zoom) + useEffect(() => { zoomRef.current = zoom }, [zoom]) useEffect(() => { const el = scrollRef.current if (!el) return const handler = (e: WheelEvent) => { if (e.ctrlKey || e.metaKey) { e.preventDefault() const delta = e.deltaY > 0 ? -0.5 : 0.5 - onZoomChange(Math.max(1, Math.min(10, zoom + delta))) + onZoomChange(Math.max(1, Math.min(10, zoomRef.current + delta))) } else { e.preventDefault() const scrubAmount = e.deltaY > 0 ? 0.5 : -0.5 - onProgressChange(Math.max(0, Math.min(100, progress + scrubAmount))) + onProgressChange(Math.max(0, Math.min(100, progressRef.current + scrubAmount))) } } el.addEventListener("wheel", handler, { passive: false }) return () => el.removeEventListener("wheel", handler) - }, [zoom, progress, onZoomChange, onProgressChange]) + }, [onZoomChange, onProgressChange])
22-26:parseOffsetsilently returns 0 for malformed input.If an offset string doesn't match the
(\d+)%pattern (e.g.,"from","to", or a bare number), it defaults to 0, which would place the keyframe at the start. Consider handling"from"/"to"aliases or logging a warning for unexpected formats.src/components/component-studio/lib/icons.tsx (1)
3-198: Consider extracting shared SVG boilerplate into a wrapper component.Every SVG icon repeats identical props (
xmlns,width,height,viewBox,fill,stroke,strokeWidth,strokeLinecap,strokeLinejoin) and most share the sameclassName. A smallIconWrappercomponent acceptingchildren(and optionally overridingclassName) would cut ~100 lines of duplication and make adding new icons trivial.♻️ Example wrapper approach
function IconSvg({ children, className = "text-zinc-400 transition-colors duration-500 group-hover:text-zinc-200", strokeWidth = 1.8, }: { children: React.ReactNode className?: string strokeWidth?: number }) { return ( <svg xmlns="http://www.w3.org/2000/svg" width={12} height={12} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={strokeWidth} strokeLinecap="round" strokeLinejoin="round" className={className} > {children} </svg> ) } // Usage: function PrismIcon() { return ( <IconSvg> <path d="M2 22 12 2l10 20" /> <path d="m3.5 19 8.5-15" /> <path d="m20.5 19-8.5-15" /> <path d="M2 22h20" /> </IconSvg> ) }src/components/ui/pill-showcase.tsx (2)
178-182: Avoid@ts-ignore— use a typed approach instead.The
@ts-ignoresuppresses a real type mismatch (assigning a string toReactNode). SincepropsForGenis already cast toRecord<string, unknown>on Line 186, you can avoid the directive entirely:♻️ Proposed fix
const propsForGen = { ...variant.props } - if (propsForGen.icon) { - // `@ts-ignore` - propsForGen.icon = "<Icon />" - } + const propsForGen: Record<string, unknown> = { ...variant.props } + if (propsForGen.icon) { + propsForGen.icon = "<Icon />" + } const code = generateComponentJsx( "HeroPill", - propsForGen as Record<string, unknown>, + propsForGen, {}, registration )
12-55:PrismIconnaming collides with the icon inicons.tsx(different SVG paths).This file defines its own
PrismIconwith different SVG geometry than the one insrc/components/component-studio/lib/icons.tsx. Having two different icons sharing the same name across the codebase will be confusing during future maintenance. Consider renaming one of them (e.g.,PrismShowcaseIconhere) to differentiate.src/components/component-studio/playground/animation-studio/animation-studio.tsx (1)
315-330:onChangeandonAddPropertydispatch identical actions.Both callbacks dispatch
SET_KEYFRAME_PROPERTYwith the same shape. If this is intentional (the reducer treats add and update identically), consider consolidating into a single callback to make that intent explicit. If the add path should differ (e.g., validation for duplicate property names), a distinct action type would be warranted.
| await Promise.all( | ||
| dbProjects.map(async project => { | ||
| if (!project.gitUrl) return | ||
| const metrics = await fetchGitMetrics(project.gitUrl) | ||
| if (metrics) { | ||
| metricsByProject[project.title] = metrics | ||
| } | ||
| }) | ||
| ) |
There was a problem hiding this comment.
Unbounded concurrent GitHub API requests may hit rate limits.
Promise.all fires fetchGitMetrics for every visible project simultaneously. On a cold cache (or revalidation), this could send dozens of concurrent requests to GitHub, risking 403 rate-limit responses — especially with unauthenticated requests (60/hr) or even authenticated ones (5000/hr shared across all endpoints).
Consider adding a concurrency limiter (e.g., processing in batches or using p-limit), or at minimum, handling per-project fetch failures gracefully so one rate-limited response doesn't reject the entire Promise.all.
Sketch: per-item error isolation
await Promise.all(
dbProjects.map(async project => {
if (!project.gitUrl) return
- const metrics = await fetchGitMetrics(project.gitUrl)
- if (metrics) {
- metricsByProject[project.title] = metrics
+ try {
+ const metrics = await fetchGitMetrics(project.gitUrl)
+ if (metrics) {
+ metricsByProject[project.title] = metrics
+ }
+ } catch (err) {
+ console.warn(`[git-metrics] Failed for ${project.title}:`, err)
}
})
)🤖 Prompt for AI Agents
In `@src/app/api/projects/git-metrics/route.ts` around lines 24 - 32, Replace the
unbounded Promise.all over dbProjects that calls fetchGitMetrics with a
bounded-concurrency or batched approach (e.g., use p-limit or process dbProjects
in chunks) to avoid hitting GitHub rate limits; ensure each call to
fetchGitMetrics is wrapped in a try/catch so failures or 403s for a single
project do not reject the whole operation, and still populate
metricsByProject[project.title] only when metrics is successfully returned.
| import { Press_Start_2P } from 'next/font/google' | ||
|
|
||
| // Toggle this to true to enable the pixel font | ||
| export const ENABLE_PIXEL_FONT = true |
There was a problem hiding this comment.
ENABLE_PIXEL_FONT is hardcoded to true — intended for production?
This merges into master with the pixel font permanently enabled. Press_Start_2P as a body font will significantly impact readability. If this is a development/playground toggle, consider defaulting to false or driving it via an environment variable so it doesn't accidentally ship.
-export const ENABLE_PIXEL_FONT = true
+export const ENABLE_PIXEL_FONT = process.env.NEXT_PUBLIC_ENABLE_PIXEL_FONT === 'true'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const ENABLE_PIXEL_FONT = true | |
| export const ENABLE_PIXEL_FONT = process.env.NEXT_PUBLIC_ENABLE_PIXEL_FONT === 'true' |
🤖 Prompt for AI Agents
In `@src/app/fonts.ts` at line 6, The constant ENABLE_PIXEL_FONT is hardcoded
true; change it to default to false and drive it from an environment/build flag
instead so it can't ship enabled accidentally—replace the literal export of
ENABLE_PIXEL_FONT with logic that reads a public env var (e.g.,
NEXT_PUBLIC_ENABLE_PIXEL_FONT or similar) and converts it to a boolean, falling
back to false when unset or invalid; ensure the flag is exposed to the client
runtime (or build-time) per your framework conventions and update any related
docs/tests that assume the pixel font is enabled.
| {animationKeyframesCSS && ( | ||
| <style dangerouslySetInnerHTML={{ __html: animationKeyframesCSS }} /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find where animationKeyframesCSS is defined and how it flows into component-preview.tsx
rg -n "animationKeyframesCSS" --type=ts --type=tsx -B 3 -A 3Repository: remcostoeten/remcostoeten.nl
Length of output: 99
🏁 Script executed:
# Also search in the parent directory structure to find how this component is used
fd . src/components/component-studio --type f --name "*.tsx" --name "*.ts" | head -20Repository: remcostoeten/remcostoeten.nl
Length of output: 322
🏁 Script executed:
# Look at the component-preview.tsx file to see full context around line 42
sed -n '1,80p' src/components/component-studio/playground/component-preview.tsxRepository: remcostoeten/remcostoeten.nl
Length of output: 2266
🏁 Script executed:
# Find where ComponentPreview is imported and used
rg "ComponentPreview" --type ts -l 2>/dev/null | head -10Repository: remcostoeten/remcostoeten.nl
Length of output: 201
🏁 Script executed:
# Search for where animationKeyframesCSS is set/assigned (excluding the component-preview file itself)
rg "animationKeyframesCSS\s*=" --type ts | head -20Repository: remcostoeten/remcostoeten.nl
Length of output: 191
🏁 Script executed:
# Find files that might generate or construct animation keyframes CSS
rg "keyframes" --type ts -l | head -15Repository: remcostoeten/remcostoeten.nl
Length of output: 451
🏁 Script executed:
# Look at playground-layout.tsx to see how animationCSS is constructed
cat -n src/components/component-studio/playground/playground-layout.tsx | head -150Repository: remcostoeten/remcostoeten.nl
Length of output: 5609
🏁 Script executed:
# Check animation-studio.tsx to understand how keyframes are generated
wc -l src/components/component-studio/playground/animation-studio/animation-studio.tsxRepository: remcostoeten/remcostoeten.nl
Length of output: 156
🏁 Script executed:
# Check animation-export.tsx to understand how CSS is built
cat -n src/components/component-studio/playground/animation-studio/animation-export.tsxRepository: remcostoeten/remcostoeten.nl
Length of output: 4775
🏁 Script executed:
# Continue reading playground-layout.tsx to find where animationCSS is constructed
sed -n '150,300p' src/components/component-studio/playground/playground-layout.tsxRepository: remcostoeten/remcostoeten.nl
Length of output: 5847
🏁 Script executed:
# Search for where animationCSS is defined/computed in playground-layout.tsx
rg -n "animationCSS" src/components/component-studio/playground/playground-layout.tsx -B 2 -A 5Repository: remcostoeten/remcostoeten.nl
Length of output: 828
🏁 Script executed:
# Look at animation-studio.tsx to understand how keyframes are created/edited (around AnimationExport usage)
grep -n "AnimationExport\|animationCSS\|keyframes" src/components/component-studio/playground/animation-studio/animation-studio.tsx | head -30Repository: remcostoeten/remcostoeten.nl
Length of output: 408
🏁 Script executed:
# Check animation-studio.tsx for UI validation or constraints on keyframe editing
sed -n '1,100p' src/components/component-studio/playground/animation-studio/animation-studio.tsxRepository: remcostoeten/remcostoeten.nl
Length of output: 2814
🏁 Script executed:
# Search for any validation or sanitization functions related to keyframes/CSS
rg "validate|sanitize|escape" src/components/component-studio/playground/ --type tsRepository: remcostoeten/remcostoeten.nl
Length of output: 54
🏁 Script executed:
# Check types.ts to see the structure of keyframes
cat -n src/components/component-studio/playground/types.ts | head -100Repository: remcostoeten/remcostoeten.nl
Length of output: 2836
🏁 Script executed:
# Check KeyframeEditor to see if there's any validation on input
cat -n src/components/component-studio/playground/animation-studio/keyframe-editor.tsx | head -150Repository: remcostoeten/remcostoeten.nl
Length of output: 5734
🏁 Script executed:
# Search for SET_KEYFRAME_PROPERTY to see how keyframe values are validated in the reducer
rg -n "SET_KEYFRAME_PROPERTY" src/components/component-studio/playground/playground-layout.tsx -B 5 -A 5Repository: remcostoeten/remcostoeten.nl
Length of output: 530
dangerouslySetInnerHTML injects user-controlled CSS values without validation.
The animationKeyframesCSS string is built directly from the keyframe editor, which allows users to input arbitrary CSS property names and values with no validation beyond trimming. Users can inject CSS attacks such as url('data:...') or SVG data URIs with embedded event handlers, enabling data exfiltration and potential XSS in some contexts.
Validate keyframe inputs before injecting into the style tag — e.g., allowlist permitted CSS properties, escape/sanitize values, or build the <style> DOM element programmatically rather than via dangerouslySetInnerHTML.
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 41-41: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.3.13)
[error] 42-42: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In `@src/components/component-studio/playground/component-preview.tsx` around
lines 41 - 43, The component currently injects user-edited CSS via
animationKeyframesCSS into a <style> tag using dangerouslySetInnerHTML; replace
this by sanitizing and/or programmatically constructing the style content:
validate keyframe inputs in the keyframe editor pipeline (allowlist permitted
CSS property names and patterns and reject or escape unsafe values like
urls/data URIs), or build the style element via DOM APIs inside ComponentPreview
(or the function that produces animationKeyframesCSS) and set textContent
instead of using dangerouslySetInnerHTML; ensure you reference and sanitize
values that feed animationKeyframesCSS and centralize the validation logic so
only safe CSS is emitted.
| // Generate a stable animation name that changes only when keyframes/bezier/duration change | ||
| // This forces the browser to re-create the animation when those values change | ||
| const animKeyframeHash = useMemo(() => { | ||
| const keyframes = state.editedKeyframes ?? activeAnimSchema?.keyframes ?? {} | ||
| const [x1, y1, x2, y2] = state.bezierValue | ||
| return JSON.stringify({ keyframes, x1, y1, x2, y2, d: state.animationDuration }) | ||
| }, [state.editedKeyframes, activeAnimSchema, state.bezierValue, state.animationDuration]) | ||
|
|
||
| useEffect(() => { | ||
| animNameRef.current = `pg-anim-${Date.now()}` | ||
| }, [animKeyframeHash]) | ||
|
|
||
| // Build animation CSS for the live preview | ||
| // Uses the negative animation-delay trick to scrub: paused animation at a specific point | ||
| const animationCSS = useMemo(() => { | ||
| if (!activeAnimSchema) return { style: undefined, css: undefined } | ||
|
|
||
| const keyframes = state.editedKeyframes ?? activeAnimSchema.keyframes | ||
| const name = animNameRef.current | ||
| const [x1, y1, x2, y2] = state.bezierValue | ||
|
|
||
| const framesCSS = Object.entries(keyframes) | ||
| .map( | ||
| ([offset, props]) => | ||
| `${offset} { ${Object.entries(props) | ||
| .map(([k, v]) => `${k}: ${v}`) | ||
| .join("; ")} }` | ||
| ) | ||
| .join("\n ") | ||
|
|
||
| const css = `@keyframes ${name} {\n ${framesCSS}\n}` | ||
|
|
||
| // Determine iteration count | ||
| const effectiveIterCount = state.repeatCount > 0 | ||
| ? state.repeatCount | ||
| : state.loop | ||
| ? "infinite" | ||
| : activeAnimSchema.iterationCount | ||
|
|
||
| const iterStr = effectiveIterCount === "infinite" | ||
| ? "infinite" | ||
| : String(effectiveIterCount) | ||
|
|
||
| if (state.isPlaying && !state.isInDelay) { | ||
| // Playing: let the animation run with CSS | ||
| const style: React.CSSProperties = { | ||
| animation: `${name} ${state.animationDuration}ms cubic-bezier(${x1}, ${y1}, ${x2}, ${y2}) ${iterStr} ${state.direction} ${state.fillMode}`, | ||
| animationPlayState: "running", | ||
| } | ||
| return { style, css } | ||
| } else { | ||
| // Paused/scrubbing: use negative animation-delay to scrub to the current progress | ||
| const delayMs = -(state.playbackProgress / 100) * state.animationDuration | ||
| const style: React.CSSProperties = { | ||
| animation: `${name} ${state.animationDuration}ms cubic-bezier(${x1}, ${y1}, ${x2}, ${y2}) 1 ${state.direction} ${state.fillMode}`, | ||
| animationPlayState: "paused", | ||
| animationDelay: `${delayMs}ms`, | ||
| } | ||
| return { style, css } | ||
| } | ||
| }, [ | ||
| activeAnimSchema, state.editedKeyframes, state.bezierValue, | ||
| state.animationDuration, state.isPlaying, state.isInDelay, | ||
| state.playbackProgress, state.loop, state.repeatCount, | ||
| state.direction, state.fillMode, animKeyframeHash, | ||
| ]) |
There was a problem hiding this comment.
Stale ref: animNameRef is updated in useEffect (post-render) but read in useMemo (during render).
When animKeyframeHash changes, the useMemo at Line 210 recalculates during the current render, reading the old animNameRef.current. The useEffect at Line 204 updates the ref after render completes. Since ref mutations don't trigger re-renders, the newly generated animation name is never picked up — defeating the intent to force the browser to re-create the @keyframes rule.
Use useState instead of useRef so the name update triggers a re-render, or derive the name directly inside the useMemo:
🔧 Option A: derive the name inside useMemo (simplest)
Remove the useRef and useEffect, and compute the name inside the useMemo:
- const animNameRef = useRef(`pg-anim-${Date.now()}`)
-
...
-
- useEffect(() => {
- animNameRef.current = `pg-anim-${Date.now()}`
- }, [animKeyframeHash])
-
const animationCSS = useMemo(() => {
if (!activeAnimSchema) return { style: undefined, css: undefined }
const keyframes = state.editedKeyframes ?? activeAnimSchema.keyframes
- const name = animNameRef.current
+ // Use a hash-derived name to force CSS re-creation on keyframe/timing changes
+ const name = `pg-anim-${btoa(animKeyframeHash).slice(0, 8).replace(/[^a-zA-Z0-9]/g, '')}`
const [x1, y1, x2, y2] = state.bezierValue🔧 Option B: use useState instead of useRef
- const animNameRef = useRef(`pg-anim-${Date.now()}`)
+ const [animName, setAnimName] = useState(`pg-anim-${Date.now()}`)
...
useEffect(() => {
- animNameRef.current = `pg-anim-${Date.now()}`
+ setAnimName(`pg-anim-${Date.now()}`)
}, [animKeyframeHash])
...
- const name = animNameRef.current
+ const name = animNameAnd add animName to the useMemo dependency array (replacing animKeyframeHash).
🤖 Prompt for AI Agents
In `@src/components/component-studio/playground/playground-layout.tsx` around
lines 196 - 261, The computed animation name stored in animNameRef (set in the
useEffect that depends on animKeyframeHash) is stale when read inside the
animationCSS useMemo because useEffect runs after render; replace this with
either (A) derive the name inside the same useMemo that builds animationCSS
(remove animNameRef and the useEffect) so the name is fresh whenever
animKeyframeHash changes, or (B) switch animNameRef to a state value (e.g.,
animName via useState and setAnimName in an effect) and include that state
(animName) in the animationCSS useMemo dependencies so the new name triggers a
re-render and the fresh name is used when building `@keyframes`; update references
to animNameRef.current to use the new local name/state.
| // Add secret auth trigger | ||
| const [clickCount, setClickCount] = useState(0) | ||
| const { openAuthModal } = useVimAuth() | ||
|
|
||
| const handleSecretAuth = () => { | ||
| const newCount = clickCount + 1 | ||
| setClickCount(newCount) | ||
|
|
||
| if (newCount === 5) { | ||
| toast.success('Secret auth triggered!') | ||
| openAuthModal() | ||
| setClickCount(0) | ||
| } | ||
|
|
||
| // Reset count if no click within 1 second | ||
| setTimeout(() => setClickCount(0), 1000) | ||
| } |
There was a problem hiding this comment.
Stale timeouts will reset click count mid-sequence.
Each click creates a new independent setTimeout that sets clickCount to 0. These are never cleared, so timeouts from earlier clicks in a sequence can fire and reset the counter while the user is still clicking. For example: 4 fast clicks, brief pause (~900ms), then resume — the first timeout fires and resets the counter to 0 before click 5.
Use a ref to track and clear the previous timeout on each click.
Proposed fix
+ import { useState, useMemo, useRef } from 'react'
- import { useState, useMemo } from 'react' // Add secret auth trigger
const [clickCount, setClickCount] = useState(0)
+ const clickTimeoutRef = useRef<ReturnType<typeof setTimeout>>(null)
const { openAuthModal } = useVimAuth()
const handleSecretAuth = () => {
+ if (clickTimeoutRef.current) {
+ clearTimeout(clickTimeoutRef.current)
+ }
+
const newCount = clickCount + 1
setClickCount(newCount)
if (newCount === 5) {
toast.success('Secret auth triggered!')
openAuthModal()
setClickCount(0)
+ return
}
// Reset count if no click within 1 second
- setTimeout(() => setClickCount(0), 1000)
+ clickTimeoutRef.current = setTimeout(() => setClickCount(0), 1000)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Add secret auth trigger | |
| const [clickCount, setClickCount] = useState(0) | |
| const { openAuthModal } = useVimAuth() | |
| const handleSecretAuth = () => { | |
| const newCount = clickCount + 1 | |
| setClickCount(newCount) | |
| if (newCount === 5) { | |
| toast.success('Secret auth triggered!') | |
| openAuthModal() | |
| setClickCount(0) | |
| } | |
| // Reset count if no click within 1 second | |
| setTimeout(() => setClickCount(0), 1000) | |
| } | |
| import { useState, useMemo, useRef } from 'react' | |
| // Add secret auth trigger | |
| const [clickCount, setClickCount] = useState(0) | |
| const clickTimeoutRef = useRef<ReturnType<typeof setTimeout>>(null) | |
| const { openAuthModal } = useVimAuth() | |
| const handleSecretAuth = () => { | |
| if (clickTimeoutRef.current) { | |
| clearTimeout(clickTimeoutRef.current) | |
| } | |
| const newCount = clickCount + 1 | |
| setClickCount(newCount) | |
| if (newCount === 5) { | |
| toast.success('Secret auth triggered!') | |
| openAuthModal() | |
| setClickCount(0) | |
| return | |
| } | |
| // Reset count if no click within 1 second | |
| clickTimeoutRef.current = setTimeout(() => setClickCount(0), 1000) | |
| } |
🤖 Prompt for AI Agents
In `@src/components/layout/footer.tsx` around lines 60 - 76, The click-count
timeout in handleSecretAuth uses independent setTimeouts that are never cleared,
causing stale timers to reset clickCount mid-sequence; fix by adding a ref
(e.g., timeoutRef via useRef<number | null>) to store the active timeout id,
call clearTimeout(timeoutRef.current) at the start of handleSecretAuth before
creating a new setTimeout, assign the new timeout id to timeoutRef.current, and
reset timeoutRef.current to null when the timeout fires or when you trigger
openAuthModal; also add a useEffect cleanup to clearTimeout(timeoutRef.current)
on unmount.
| export function ProjectShowcase({ visibleRowCount = 6 }: Props) { | ||
| const { data, isLoading } = useQuery({ | ||
| queryKey: ['project-showcase'], | ||
| queryFn: fetchProjectShowcaseData, | ||
| staleTime: 5 * 60 * 1000, | ||
| gcTime: 30 * 60 * 1000, | ||
| retry: 1, | ||
| refetchOnWindowFocus: false | ||
| }) |
There was a problem hiding this comment.
No error state — failed fetch results in an infinite skeleton.
If fetchProjectShowcaseData fails (after 1 retry), useQuery sets isLoading: false and data remains undefined. The guard on line 93 (isLoading || !data) will keep rendering the skeleton indefinitely with no error feedback to the user.
Consider destructuring isError/error from useQuery and rendering an error fallback:
Proposed fix
- const { data, isLoading } = useQuery({
+ const { data, isLoading, isError } = useQuery({
queryKey: ['project-showcase'],
queryFn: fetchProjectShowcaseData,
staleTime: 5 * 60 * 1000,
gcTime: 30 * 60 * 1000,
retry: 1,
refetchOnWindowFocus: false
})
...
- if (isLoading || !data) {
+ if (isLoading || !data) {
+ return <ProjectShowcaseSkeleton visibleRowCount={visibleRowCount} />
+ }
+
+ if (isError) {
+ return null // or a minimal error message
+ }Also applies to: 93-95
🤖 Prompt for AI Agents
In `@src/components/projects/components/project-showcase.tsx` around lines 36 -
44, Destructure isError, error and refetch from the useQuery call in
ProjectShowcase (alongside data and isLoading) and change the render guard so
the skeleton shows only while isLoading; when isError or data is undefined
render an error fallback UI that displays error?.message (and a retry button
that calls refetch) instead of the skeleton. Specifically update the useQuery
call that uses fetchProjectShowcaseData to include isError/error/refetch and
replace the current "isLoading || !data" check with an ordered conditional: if
(isLoading) -> skeleton, else if (isError || !data) -> error fallback (show
error message and retry), else -> render the normal project grid.
| export function WebVitalsReporter() { | ||
| if (process.env.NODE_ENV !== 'development') { | ||
| return null | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| const logMetric = (metric: Metric) => { | ||
| let cancelled = false | ||
|
|
||
| const setup = async () => { | ||
| const { onCLS, onFCP, onLCP, onTTFB } = await import( | ||
| 'web-vitals' | ||
| ) | ||
| if (cancelled) return | ||
|
|
||
| const logMetric = (metric: Metric) => { | ||
| console.log( | ||
| `%c${metric.name}%c ${formatValue(metric.name, metric.value)} ${getEmoji(metric.rating)}`, | ||
| 'font-weight: bold; color: #10b981', | ||
| 'color: inherit' | ||
| ) | ||
| } | ||
|
|
||
| console.log( | ||
| `%c${metric.name}%c ${formatValue(metric.name, metric.value)} ${getEmoji(metric.rating)}`, | ||
| 'font-weight: bold; color: #10b981', | ||
| 'color: inherit' | ||
| '%c📊 Web Vitals Reporter Active', | ||
| 'font-size: 14px; font-weight: bold' | ||
| ) | ||
| } | ||
|
|
||
| console.log( | ||
| '%c📊 Web Vitals Reporter Active', | ||
| 'font-size: 14px; font-weight: bold' | ||
| ) | ||
| onLCP(logMetric) | ||
| onFCP(logMetric) | ||
| onCLS(logMetric) | ||
| onTTFB(logMetric) | ||
| } | ||
|
|
||
| onLCP(logMetric) | ||
| onFCP(logMetric) | ||
| onCLS(logMetric) | ||
| onTTFB(logMetric) | ||
| void setup() | ||
| return () => { | ||
| cancelled = true | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
Hooks violation: useEffect called after early return.
The early return on lines 30–32 causes useEffect on line 34 to be skipped in non-development builds, violating the Rules of Hooks. Biome correctly flags this. Even though NODE_ENV is a build-time constant, this pattern is fragile and breaks linting. Move the guard inside the effect.
Proposed fix
export function WebVitalsReporter() {
- if (process.env.NODE_ENV !== 'development') {
- return null
- }
-
useEffect(() => {
+ if (process.env.NODE_ENV !== 'development') return
+
let cancelled = false
const setup = async () => {
@@ ...
void setup()
return () => {
cancelled = true
}
}, [])
- return null
+ return null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function WebVitalsReporter() { | |
| if (process.env.NODE_ENV !== 'development') { | |
| return null | |
| } | |
| useEffect(() => { | |
| const logMetric = (metric: Metric) => { | |
| let cancelled = false | |
| const setup = async () => { | |
| const { onCLS, onFCP, onLCP, onTTFB } = await import( | |
| 'web-vitals' | |
| ) | |
| if (cancelled) return | |
| const logMetric = (metric: Metric) => { | |
| console.log( | |
| `%c${metric.name}%c ${formatValue(metric.name, metric.value)} ${getEmoji(metric.rating)}`, | |
| 'font-weight: bold; color: #10b981', | |
| 'color: inherit' | |
| ) | |
| } | |
| console.log( | |
| `%c${metric.name}%c ${formatValue(metric.name, metric.value)} ${getEmoji(metric.rating)}`, | |
| 'font-weight: bold; color: #10b981', | |
| 'color: inherit' | |
| '%c📊 Web Vitals Reporter Active', | |
| 'font-size: 14px; font-weight: bold' | |
| ) | |
| } | |
| console.log( | |
| '%c📊 Web Vitals Reporter Active', | |
| 'font-size: 14px; font-weight: bold' | |
| ) | |
| onLCP(logMetric) | |
| onFCP(logMetric) | |
| onCLS(logMetric) | |
| onTTFB(logMetric) | |
| } | |
| onLCP(logMetric) | |
| onFCP(logMetric) | |
| onCLS(logMetric) | |
| onTTFB(logMetric) | |
| void setup() | |
| return () => { | |
| cancelled = true | |
| } | |
| }, []) | |
| export function WebVitalsReporter() { | |
| useEffect(() => { | |
| if (process.env.NODE_ENV !== 'development') return | |
| let cancelled = false | |
| const setup = async () => { | |
| const { onCLS, onFCP, onLCP, onTTFB } = await import( | |
| 'web-vitals' | |
| ) | |
| if (cancelled) return | |
| const logMetric = (metric: Metric) => { | |
| console.log( | |
| `%c${metric.name}%c ${formatValue(metric.name, metric.value)} ${getEmoji(metric.rating)}`, | |
| 'font-weight: bold; color: `#10b981`', | |
| 'color: inherit' | |
| ) | |
| } | |
| console.log( | |
| '%c📊 Web Vitals Reporter Active', | |
| 'font-size: 14px; font-weight: bold' | |
| ) | |
| onLCP(logMetric) | |
| onFCP(logMetric) | |
| onCLS(logMetric) | |
| onTTFB(logMetric) | |
| } | |
| void setup() | |
| return () => { | |
| cancelled = true | |
| } | |
| }, []) | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 34-34: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In `@src/components/seo/web-vitals-reporter.tsx` around lines 29 - 66, The
component WebVitalsReporter currently returns early before calling useEffect
which violates the Rules of Hooks; remove the top-level NODE_ENV early return so
useEffect is always invoked, then move the development-mode guard inside the
effect (at the start of the effect callback) to bail out immediately if
process.env.NODE_ENV !== 'development'; keep the existing setup async function,
cancelled flag, logMetric, and cleanup logic (returning the cleanup that sets
cancelled) and maintain the empty dependency array so onLCP/onFCP/onCLS/onTTFB
are only registered in development builds.
| const effectiveVariant: PillVariant = | ||
| variant === "ghost" && ghostBehavior === "always" | ||
| ? "ghost" | ||
| : variant === "ghost" && ghostBehavior === "idle" && isIdle | ||
| ? "ghost" | ||
| : variant |
There was a problem hiding this comment.
effectiveVariant logic doesn't correctly suppress ghost when idle is false.
When variant="ghost" and ghostBehavior="idle", the intent is to show ghost styling only while idle. However, when isIdle is false, the ternary falls through to the final branch which returns variant — still "ghost". The same issue applies to ghostBehavior="never" with variant="ghost" — it always renders as ghost.
The fallback should return a non-ghost variant (e.g., "default") when the ghost effect is inactive:
Proposed fix
const effectiveVariant: PillVariant =
- variant === "ghost" && ghostBehavior === "always"
- ? "ghost"
- : variant === "ghost" && ghostBehavior === "idle" && isIdle
- ? "ghost"
- : variant
+ ghostBehavior === "always"
+ ? "ghost"
+ : ghostBehavior === "idle" && isIdle
+ ? "ghost"
+ : ghostBehavior === "never"
+ ? variant
+ : variant === "ghost"
+ ? "default"
+ : variantAlternatively, consider accepting a separate activeVariant prop so the non-ghost appearance is explicitly configurable rather than hardcoded to "default".
🤖 Prompt for AI Agents
In `@src/components/ui/hero-pill.tsx` around lines 46 - 51, The effectiveVariant
computation incorrectly falls back to the original variant (which may be
"ghost") when the ghost effect is inactive; update the logic in effectiveVariant
(and the PillVariant usage) so that when variant === "ghost" but ghostBehavior
is "idle" and isIdle is false, or when ghostBehavior is "never", it returns a
non-ghost variant (e.g., "default") instead of variant; implement this by
changing the ternary to explicitly return "default" (or use a new prop
activeVariant to allow callers to override the non-ghost appearance) whenever
the ghost behavior is not active, and update any call sites that may rely on the
old fallback.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores