Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions apps/webapp/app/components/ErrorDisplay.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { HomeIcon } from "@heroicons/react/20/solid";
import { isRouteErrorResponse, useRouteError } from "@remix-run/react";
import { motion } from "framer-motion";
import { friendlyErrorDisplay } from "~/utils/httpErrors";
import { LinkButton } from "./primitives/Buttons";
import { Header1 } from "./primitives/Headers";
import { Paragraph } from "./primitives/Paragraph";
import Spline from "@splinetool/react-spline";
import { TriggerRotatingLogo } from "./TriggerRotatingLogo";
import { type ReactNode } from "react";

type ErrorDisplayOptions = {
Expand Down Expand Up @@ -57,14 +56,7 @@ export function ErrorDisplay({ title, message, button }: DisplayOptionsProps) {
{button ? button.title : "Go to homepage"}
</LinkButton>
</div>
<motion.div
className="pointer-events-none absolute inset-0 overflow-hidden"
initial={{ opacity: 0 }}
animate={{ opacity: 1 }}
transition={{ delay: 0.5, duration: 2, ease: "easeOut" }}
>
<Spline scene="https://prod.spline.design/wRly8TZN-e0Twb8W/scene.splinecode" />
</motion.div>
<TriggerRotatingLogo />
</div>
);
}
75 changes: 75 additions & 0 deletions apps/webapp/app/components/TriggerRotatingLogo.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { motion } from "framer-motion";
import { useEffect, useState } from "react";

declare global {
namespace JSX {
interface IntrinsicElements {
"spline-viewer": React.DetailedHTMLProps<
React.HTMLAttributes<HTMLElement> & {
url?: string;
"loading-anim-type"?: string;
},
HTMLElement
>;
}
}

interface Window {
__splineLoader?: Promise<void>;
}
}

export function TriggerRotatingLogo() {
const [isSplineReady, setIsSplineReady] = useState(false);

useEffect(() => {
// Already registered from a previous render
if (customElements.get("spline-viewer")) {
setIsSplineReady(true);
return;
}

// Another mount already started loading - share the same promise
if (window.__splineLoader) {
window.__splineLoader.then(() => setIsSplineReady(true)).catch(() => setIsSplineReady(false));
return;
}

// First mount: create script and shared loader promise
const script = document.createElement("script");
script.type = "module";
// Version pinned; SRI hash omitted as unpkg doesn't guarantee hash stability across deploys
script.src = "https://unpkg.com/@splinetool/viewer@1.12.29/build/spline-viewer.js";

window.__splineLoader = new Promise<void>((resolve, reject) => {
script.onload = () => resolve();
script.onerror = () => reject();
});

window.__splineLoader.then(() => setIsSplineReady(true)).catch(() => setIsSplineReady(false));

document.head.appendChild(script);

// Intentionally no cleanup: once the custom element is registered globally,
// removing the script would break re-mounts while providing no benefit
}, []);
Comment on lines +25 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Race condition successfully addressed; prevent setState after unmount.

The shared window.__splineLoader promise elegantly solves the race condition flagged in the previous review. However, the promise handlers on lines 34 and 49 call setIsSplineReady without checking if the component is still mounted, which could cause warnings or unexpected behavior if the component unmounts before the script loads.

🔧 Proposed fix using cleanup pattern
 export function TriggerRotatingLogo() {
   const [isSplineReady, setIsSplineReady] = useState(false);
 
   useEffect(() => {
+    let mounted = true;
+
     // Already registered from a previous render
     if (customElements.get("spline-viewer")) {
       setIsSplineReady(true);
       return;
     }
 
     // Another mount already started loading - share the same promise
     if (window.__splineLoader) {
-      window.__splineLoader.then(() => setIsSplineReady(true)).catch(() => setIsSplineReady(false));
+      window.__splineLoader
+        .then(() => mounted && setIsSplineReady(true))
+        .catch(() => mounted && setIsSplineReady(false));
       return;
     }
 
     // First mount: create script and shared loader promise
     const script = document.createElement("script");
     script.type = "module";
     // Version pinned; SRI hash omitted as unpkg doesn't guarantee hash stability across deploys
     script.src = "https://unpkg.com/@splinetool/viewer@1.12.29/build/spline-viewer.js";
 
     window.__splineLoader = new Promise<void>((resolve, reject) => {
       script.onload = () => resolve();
       script.onerror = () => reject();
     });
 
-    window.__splineLoader.then(() => setIsSplineReady(true)).catch(() => setIsSplineReady(false));
+    window.__splineLoader
+      .then(() => mounted && setIsSplineReady(true))
+      .catch(() => mounted && setIsSplineReady(false));
 
     document.head.appendChild(script);
 
     // Intentionally no cleanup: once the custom element is registered globally,
     // removing the script would break re-mounts while providing no benefit
+
+    return () => {
+      mounted = false;
+    };
   }, []);
🤖 Prompt for AI Agents
In @apps/webapp/app/components/TriggerRotatingLogo.tsx around lines 25 - 55, The
promise handlers in useEffect (window.__splineLoader.then(...) and the
onload/onerror handlers that call setIsSplineReady) can call setState after the
component unmounts; fix by adding a mounted flag inside the useEffect (e.g., let
mounted = true), replace direct setIsSplineReady(...) calls with guarded calls
(if (mounted) setIsSplineReady(true/false)), and return a cleanup function that
sets mounted = false so any late-resolving window.__splineLoader or script
callbacks won't update state after unmount; ensure you update both places where
setIsSplineReady is used (the existing window.__splineLoader.then/catch and the
script.onload/script.onerror resolver/rejector handlers).


if (!isSplineReady) {
return null;
}

return (
<motion.div
className="pointer-events-none absolute inset-0 overflow-hidden"
initial={{ opacity: 0 }}
animate={{ opacity: 1 }}
transition={{ delay: 0.5, duration: 2, ease: "easeOut" }}
>
<spline-viewer
loading-anim-type="spinner-small-light"
url="https://prod.spline.design/wRly8TZN-e0Twb8W/scene.splinecode"
style={{ width: "100%", height: "100%" }}
/>
</motion.div>
);
}
1 change: 0 additions & 1 deletion apps/webapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@
"@sentry/remix": "9.46.0",
"@slack/web-api": "7.9.1",
"@socket.io/redis-adapter": "^8.3.0",
"@splinetool/react-spline": "^2.2.6",
"@tabler/icons-react": "^2.39.0",
"@tailwindcss/container-queries": "^0.1.1",
"@tanstack/react-virtual": "^3.0.4",
Expand Down
42 changes: 0 additions & 42 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.