Skip to content
Closed
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
114 changes: 114 additions & 0 deletions src/components/IframeCache/IframeCache.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { createContext, useCallback, useContext, useEffect, useMemo, useRef, type ReactNode } from 'react';
import { createPortal } from 'react-dom';

interface IframeCacheContextValue {
getIframeContainer: (key: string) => HTMLDivElement;
releaseIframeContainer: (key: string) => void;
}

const IframeCacheContext = createContext<IframeCacheContextValue | null>(null);

export const useIframeCache = () => {

Check warning on line 11 in src/components/IframeCache/IframeCache.tsx

View workflow job for this annotation

GitHub Actions / Run Linter

Fast refresh only works when a file only exports components. Use a new file to share constants or functions between components
const context = useContext(IframeCacheContext);
if (!context) {
throw new Error('useIframeCache must be used within IframeCacheProvider');
}
return context;
};

interface IframeCacheProviderProps {
readonly children: ReactNode;
}

export function IframeCacheProvider({ children }: IframeCacheProviderProps) {
const containerMapRef = useRef<Map<string, HTMLDivElement>>(new Map());
Comment thread
QuentinVdr marked this conversation as resolved.
const lastUsedRef = useRef<Map<string, number>>(new Map());
const hiddenContainerRef = useRef<HTMLDivElement | null>(null);

useEffect(() => {
// Create hidden container for cached iframes
const hiddenDiv = document.createElement('div');
hiddenDiv.style.position = 'fixed';
hiddenDiv.style.top = '-9999px';
hiddenDiv.style.left = '-9999px';
hiddenDiv.style.width = '0';
hiddenDiv.style.height = '0';
hiddenDiv.style.overflow = 'hidden';
hiddenDiv.style.pointerEvents = 'none';
document.body.appendChild(hiddenDiv);
hiddenContainerRef.current = hiddenDiv;

const cleanupInterval = setInterval(
() => {
const now = Date.now();
const CACHE_TTL = 10 * 60 * 1000; // 10 minutes

for (const [key, lastUsed] of lastUsedRef.current.entries()) {
if (now - lastUsed > CACHE_TTL) {
const container = containerMapRef.current.get(key);
container?.remove();
containerMapRef.current.delete(key);
lastUsedRef.current.delete(key);
}
}
},
5 * 60 * 1000
);
Comment on lines +41 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Cleanup may remove actively used containers.

The cleanup interval removes containers based solely on the lastUsed timestamp, without checking whether the container is currently visible (mounted in a target element). If a user keeps a stream open for more than 10 minutes, its container will be removed from the map and DOM while still in use, breaking the UI.

Apply this diff to only evict containers that are in the hidden container:

     const cleanupInterval = setInterval(
       () => {
         const now = Date.now();
         const CACHE_TTL = 10 * 60 * 1000; // 10 minutes
 
         for (const [key, lastUsed] of lastUsedRef.current.entries()) {
           if (now - lastUsed > CACHE_TTL) {
             const container = containerMapRef.current.get(key);
-            container?.remove();
-            containerMapRef.current.delete(key);
-            lastUsedRef.current.delete(key);
+            // Only remove if container is in the hidden area (not actively displayed)
+            if (container && container.parentElement === hiddenContainerRef.current) {
+              container.remove();
+              containerMapRef.current.delete(key);
+              lastUsedRef.current.delete(key);
+            }
           }
         }
       },
       5 * 60 * 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.

Suggested change
const cleanupInterval = setInterval(
() => {
const now = Date.now();
const CACHE_TTL = 10 * 60 * 1000; // 10 minutes
for (const [key, lastUsed] of lastUsedRef.current.entries()) {
if (now - lastUsed > CACHE_TTL) {
const container = containerMapRef.current.get(key);
container?.remove();
containerMapRef.current.delete(key);
lastUsedRef.current.delete(key);
}
}
},
5 * 60 * 1000
);
const cleanupInterval = setInterval(
() => {
const now = Date.now();
const CACHE_TTL = 10 * 60 * 1000; // 10 minutes
for (const [key, lastUsed] of lastUsedRef.current.entries()) {
if (now - lastUsed > CACHE_TTL) {
const container = containerMapRef.current.get(key);
// Only remove if container is in the hidden area (not actively displayed)
if (container && container.parentElement === hiddenContainerRef.current) {
container.remove();
containerMapRef.current.delete(key);
lastUsedRef.current.delete(key);
}
}
}
},
5 * 60 * 1000
);
🤖 Prompt for AI Agents
In src/components/IframeCache/IframeCache.tsx around lines 41 to 56, the cleanup
currently evicts containers purely by lastUsed timestamp which can remove
containers that are actively mounted; modify the eviction to also check that the
container is inside the hidden container before removing: obtain the hidden
container element (hiddenContainerRef.current) and only call container.remove()
and delete from maps when container !== undefined AND hiddenContainerRef.current
exists AND container.parentElement === hiddenContainerRef.current (i.e.,
container is currently in the hidden/offscreen pool); keep the existing TTL and
interval logic unchanged.


return () => {
hiddenDiv.remove();
clearInterval(cleanupInterval);
};
}, []);

const getIframeContainer = useCallback((key: string): HTMLDivElement => {
lastUsedRef.current.set(key, Date.now());
if (!containerMapRef.current.has(key)) {
const container = document.createElement('div');
container.style.width = '100%';
container.style.height = '100%';
containerMapRef.current.set(key, container);
}
return containerMapRef.current.get(key)!;
Comment thread
QuentinVdr marked this conversation as resolved.
}, []);

const releaseIframeContainer = useCallback((key: string) => {
const container = containerMapRef.current.get(key);
if (container) {
// Move to hidden container to keep iframe alive
hiddenContainerRef.current?.appendChild(container);
}
}, []);

const value = useMemo(
() => ({ getIframeContainer, releaseIframeContainer }),
[getIframeContainer, releaseIframeContainer]
);

return <IframeCacheContext.Provider value={value}>{children}</IframeCacheContext.Provider>;
}

interface CachedIframePortalProps {
cacheKey: string;
targetRef: React.RefObject<HTMLDivElement | null>;
children: ReactNode;
}

export function CachedIframePortal({ cacheKey, targetRef, children }: CachedIframePortalProps) {
const { getIframeContainer, releaseIframeContainer } = useIframeCache();
const container = getIframeContainer(cacheKey);

useEffect(() => {
// Move container to target when mounted
if (targetRef.current) {
targetRef.current.appendChild(container);
}

return () => {
// Release container when unmounted
releaseIframeContainer(cacheKey);
};
}, [cacheKey, container, targetRef, releaseIframeContainer]);

return createPortal(children, container);
}
Comment on lines +97 to +114

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Side effects during render and ref tracking issues.

Multiple issues with the portal implementation:

  1. Line 99: getIframeContainer is called during render, not in an effect. This creates side effects during render (updating lastUsed ref) and violates React's rendering model. It should be moved into the effect.

  2. Effect dependencies: The effect depends on targetRef, but React doesn't track changes to targetRef.current. If the ref's current value changes after mount, the container won't be re-attached to the new target.

  3. Null safety: If targetRef.current is null on mount, the container won't be appended anywhere, leaving the portal in an inconsistent state.

Apply this diff to fix these issues:

 export function CachedIframePortal({ cacheKey, targetRef, children }: CachedIframePortalProps) {
   const { getIframeContainer, releaseIframeContainer } = useIframeCache();
-  const container = getIframeContainer(cacheKey);
+  const containerRef = useRef<HTMLDivElement | null>(null);
 
   useEffect(() => {
+    // Get or create container inside effect
+    const container = getIframeContainer(cacheKey);
+    containerRef.current = container;
+
     // Move container to target when mounted
-    if (targetRef.current) {
+    const target = targetRef.current;
+    if (target) {
-      targetRef.current.appendChild(container);
+      target.appendChild(container);
+    } else {
+      console.warn(`CachedIframePortal: targetRef.current is null for key "${cacheKey}"`);
     }
 
     return () => {
       // Release container when unmounted
       releaseIframeContainer(cacheKey);
     };
   }, [cacheKey, targetRef, getIframeContainer, releaseIframeContainer]);
 
-  return createPortal(children, container);
+  return containerRef.current ? createPortal(children, containerRef.current) : 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.

Suggested change
export function CachedIframePortal({ cacheKey, targetRef, children }: CachedIframePortalProps) {
const { getIframeContainer, releaseIframeContainer } = useIframeCache();
const container = getIframeContainer(cacheKey);
useEffect(() => {
// Move container to target when mounted
if (targetRef.current) {
targetRef.current.appendChild(container);
}
return () => {
// Release container when unmounted
releaseIframeContainer(cacheKey);
};
}, [cacheKey, container, targetRef, releaseIframeContainer]);
return createPortal(children, container);
}
export function CachedIframePortal({ cacheKey, targetRef, children }: CachedIframePortalProps) {
const { getIframeContainer, releaseIframeContainer } = useIframeCache();
const containerRef = useRef<HTMLDivElement | null>(null);
useEffect(() => {
// Get or create container inside effect
const container = getIframeContainer(cacheKey);
containerRef.current = container;
// Move container to target when mounted
const target = targetRef.current;
if (target) {
target.appendChild(container);
} else {
console.warn(`CachedIframePortal: targetRef.current is null for key "${cacheKey}"`);
}
return () => {
// Release container when unmounted
releaseIframeContainer(cacheKey);
};
}, [cacheKey, targetRef, getIframeContainer, releaseIframeContainer]);
return containerRef.current
? createPortal(children, containerRef.current)
: null;
}
🤖 Prompt for AI Agents
In src/components/IframeCache/IframeCache.tsx around lines 97-114, move the call
to getIframeContainer out of render and into a useEffect: create a local state
(or ref) to hold the container and initialize it inside useEffect using
getIframeContainer(cacheKey), then only render the portal when that container
exists. In the same effect (or a separate effect) append the container to
targetRef.current and re-attach if targetRef.current changes by tracking the
actual DOM node (use a ref for previousTarget or a callback ref) so changes to
targetRef.current re-run the attach logic; ensure cleanup releases the container
via releaseIframeContainer(cacheKey) and removes the node from any previous
target to avoid leaks. Also handle the case where targetRef.current is null on
mount by still creating the container and waiting to append it later when
targetRef.current becomes non-null (i.e., perform append on any target change),
keeping createPortal conditional on the container being non-null.

3 changes: 2 additions & 1 deletion src/components/gridItems/ChatItem/ChatItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ const ChatItem = memo(
({ streamName, isDarkThemePreferred = false }: ChatItemProps) => {
const title = `Chat: ${streamName}`;
const iframeSrc = `https://www.twitch.tv/embed/${streamName}/chat?parent=${window.location.hostname}${isDarkThemePreferred ? '&darkpopout' : ''}`;
const cacheKey = `chat-${streamName}`;

return (
<GridItem title={title} iframeSrc={iframeSrc} streamName={streamName}>
<GridItem title={title} iframeSrc={iframeSrc} streamName={streamName} cacheKey={cacheKey}>
<ChatItemHeader streamName={streamName} />
</GridItem>
);
Expand Down
35 changes: 21 additions & 14 deletions src/components/gridItems/GridItem/GridItem.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { CachedIframePortal } from '@/components/IframeCache/IframeCache';
import { useIframeRefresh } from '@/hooks/useIframeRefresh';
import { forwardRef, memo, useImperativeHandle, type ReactNode } from 'react';
import { forwardRef, memo, useImperativeHandle, useRef, type ReactNode } from 'react';
import GridItemHeader from './GridItemHeader/GridItemHeader';

interface GridItemProps {
Expand All @@ -9,6 +10,7 @@ interface GridItemProps {
streamName?: string;
onRefresh?: () => void;
children?: ReactNode;
cacheKey: string;
Comment thread
QuentinVdr marked this conversation as resolved.
}

export interface GridItemRef {
Expand All @@ -17,8 +19,9 @@ export interface GridItemRef {

const GridItem = memo(
forwardRef<GridItemRef, GridItemProps>(
({ title, iframeSrc, allowFullScreen, streamName, onRefresh, children }, ref) => {
({ title, iframeSrc, allowFullScreen, streamName, onRefresh, children, cacheKey }, ref) => {
const { iframeRef, refreshIframe } = useIframeRefresh(onRefresh);
const iframeContainerRef = useRef<HTMLDivElement>(null);

useImperativeHandle(ref, () => ({ refreshIframe }), [refreshIframe]);

Expand All @@ -30,18 +33,22 @@ const GridItem = memo(
<GridItemHeader title={title} streamName={streamName}>
{children}
</GridItemHeader>
<iframe
ref={iframeRef}
key={iframeSrc}
title={title}
src={iframeSrc}
height="100%"
width="100%"
allowFullScreen={allowFullScreen}
loading="lazy"
className="transition-all duration-300 ease-in-out group-hover:mt-12"
style={{ transform: 'translateZ(0)' }}
/>
<div ref={iframeContainerRef} className="relative h-full w-full">
<CachedIframePortal cacheKey={cacheKey} targetRef={iframeContainerRef}>
<iframe
ref={iframeRef}
key={iframeSrc}
title={title}
src={iframeSrc}
height="100%"
width="100%"
allowFullScreen={allowFullScreen}
loading="lazy"
className="transition-all duration-300 ease-in-out group-hover:mt-12"
style={{ transform: 'translateZ(0)' }}
/>
</CachedIframePortal>
</div>
</section>
);
}
Expand Down
10 changes: 9 additions & 1 deletion src/components/gridItems/StreamItem/StreamItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,21 @@ const StreamItem = memo(
const title = `Stream: ${streamName}`;

const iframeSrc = `https://player.twitch.tv/?channel=${streamName}&parent=${window.location.hostname}`;
const cacheKey = `stream-${streamName}`;

const handleRefresh = useCallback(() => {
gridItemRef.current?.refreshIframe();
}, []);

return (
<GridItem ref={gridItemRef} title={title} iframeSrc={iframeSrc} allowFullScreen streamName={streamName}>
<GridItem
ref={gridItemRef}
title={title}
iframeSrc={iframeSrc}
allowFullScreen
streamName={streamName}
cacheKey={cacheKey}
>
<StreamItemHeader streamName={streamName} handleRefresh={handleRefresh} />
</GridItem>
);
Expand Down
79 changes: 41 additions & 38 deletions src/routes/watch.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ChatItem from '@/components/gridItems/ChatItem/ChatItem';
import StreamItem from '@/components/gridItems/StreamItem/StreamItem';
import GridToolsBar from '@/components/GridToolsBar/GridToolsBar';
import { IframeCacheProvider } from '@/components/IframeCache/IframeCache';
import useWindowDimensions from '@/hooks/useWindowDimensions';
import { useStreamStore } from '@/stores/streamStore';
import { createFileRoute, redirect, useNavigate } from '@tanstack/react-router';
Expand Down Expand Up @@ -55,46 +56,48 @@ function Watch() {
const rowHeight = useMemo(() => Math.max(24, Math.floor(dimensions.height / 12)), [dimensions.height]);

return (
<div className="min-h-dvh w-full">
<GridToolsBar />
<ReactGridLayout
className="layout"
layout={layout}
cols={12}
rowHeight={rowHeight}
width={dimensions.hasVerticalScrollbar ? dimensions.width - 1 : dimensions.width}
isDraggable
isResizable
resizeHandles={['se', 'sw', 'ne', 'nw']}
margin={[0, 0]}
containerPadding={[0, 0]}
useCSSTransforms
transformScale={1}
verticalCompact
onLayoutChange={handleLayoutChange}
>
{layout.map(item => {
const [type, streamName] = item.i.split('-', 2);
<IframeCacheProvider>
<div className="min-h-dvh w-full">
<GridToolsBar />
<ReactGridLayout
className="layout"
layout={layout}
cols={12}
rowHeight={rowHeight}
width={dimensions.hasVerticalScrollbar ? dimensions.width - 1 : dimensions.width}
isDraggable
isResizable
resizeHandles={['se', 'sw', 'ne', 'nw']}
margin={[0, 0]}
containerPadding={[0, 0]}
useCSSTransforms
transformScale={1}
verticalCompact
onLayoutChange={handleLayoutChange}
>
{layout.map(item => {
const [type, streamName] = item.i.split('-', 2);

if (type === 'stream') {
return (
<div key={item.i}>
<StreamItem streamName={streamName} />
</div>
);
}
if (type === 'stream') {
return (
<div key={item.i}>
<StreamItem streamName={streamName} />
</div>
);
}

if (type === 'chat') {
return (
<div key={item.i} className={isActiveChat(streamName) ? '' : 'hidden'}>
<ChatItem streamName={streamName} isDarkThemePreferred={isDarkThemePreferred} />
</div>
);
}
if (type === 'chat') {
return (
<div key={item.i} className={isActiveChat(streamName) ? '' : 'hidden'}>
<ChatItem streamName={streamName} isDarkThemePreferred={isDarkThemePreferred} />
</div>
);
}

return null;
})}
</ReactGridLayout>
</div>
return null;
})}
</ReactGridLayout>
</div>
</IframeCacheProvider>
);
}