-
Notifications
You must be signed in to change notification settings - Fork 0
few more changes #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces multi-platform video quality selection, network-aware playback optimization, component memoization with React Query prefetching, CSS-based animations replacing Framer Motion, Kick API date normalization, and build tooling enhancements for compression and bundle analysis. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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
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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
streamstorm | 13563a3 | Jan 19 2026, 12:01 AM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (2)
vite.renderer.config.ts (1)
117-117: Remove staleframer-motionfromoptimizeDeps.include.
framer-motionwas removed frompackage.jsondependencies but is still listed inoptimizeDeps.include. This will cause Vite to attempt pre-bundling a non-existent package, resulting in warnings or errors during development.Proposed fix
include: [ 'react', 'react-dom', '@tanstack/react-query', '@tanstack/react-router', 'zustand', 'hls.js', - 'framer-motion', 'clsx', 'tailwind-merge', 'class-variance-authority', 'lucide-react', ],src/components/stream/stream-info.tsx (1)
176-182: Handle live streams with missingstartedAtto avoid empty uptime display.If
startedAtis null (allowed by the type), the UI shows only the clock icon. Consider a fallback value to keep the layout stable.💡 Proposed fix
- {stream.startedAt && <UptimeCounter startedAt={stream.startedAt} />} + {stream.startedAt ? ( + <UptimeCounter startedAt={stream.startedAt} /> + ) : ( + <span className="font-semibold tabular-nums text-white">0:00:00</span> + )}
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 57-61: The dependency "vite-plugin-compression" may not be proven
compatible with Vite 7; verify compatibility by running a full dev/build cycle
with the current Vite version (the "vite" dependency) and watch for
plugin-related errors, then either upgrade "vite-plugin-compression" to a
Vite‑7‑compatible release or replace it with an alternative plugin if you
encounter API breakage; check and, if needed, update any usage sites (plugin
configuration in vite.config.* and any calls to compression() options) to match
the compatible plugin API before merging.
In `@src/backend/api/platforms/kick/kick-transformers.ts`:
- Around line 60-79: The timezone-detection logic in normalizeKickDate
incorrectly treats ISO strings with negative offsets as lacking timezone info;
update the branch that currently checks "dateStr.includes('T') &&
!dateStr.endsWith('Z') && !dateStr.includes('+')" to detect any timezone offset
(both '+' and '-') instead of only '+'. Concretely, in normalizeKickDate replace
the simple '+' check with a robust check for timezone suffixes (e.g., a regex
that matches 'Z' or a +/− offset like ±HH or ±HH:MM at the end) so strings like
"2024-01-01T12:00:00-05:00" are not appended with 'Z' and only truly
timezone-less strings get 'Z' appended.
🧹 Nitpick comments (5)
src/components/stream/stream-card.tsx (1)
18-53: Good memoization and dual prefetch implementation.The hover-based prefetching for both channel and stream data is a solid UX optimization. The channel prefetch correctly sets
staleTime: 5 * 60 * 1000.Consider adding
staleTimeto the stream prefetch as well for consistency, though stream data may intentionally need fresher updates.Optional: Add staleTime to stream prefetch
// Prefetch stream data queryClient.prefetchQuery({ queryKey: STREAM_KEYS.byChannel(stream.channelName, stream.platform), queryFn: async () => { const response = await window.electronAPI.streams.getByChannel({ username: stream.channelName, platform: stream.platform, }); if (response.error) throw new Error(response.error); return response.data; }, + staleTime: 1000 * 60, // 1 minute - streams change more frequently });src/index.css (1)
66-132: Add reduced‑motion overrides for accessibility.
These animations are applied broadly; consider disabling them when users opt out of motion.♿ Suggested CSS override
@@ .stagger-scale-container > *:nth-child(n+13) { animation-delay: 0.2s; } + +@media (prefers-reduced-motion: reduce) { + .animate-fade-in-up, + .animate-fade-in-scale, + .stagger-container > *, + .stagger-scale-container > * { + animation: none !important; + } +}src/components/player/twitch/twitch-vod-player.tsx (1)
30-30: Consider aligning type withStreamPlayback.qualities.The type here
{ quality: string, url: string }[]is missing the optionalframeRatefield that exists inStreamPlayback.qualities. For consistency across the codebase, consider using the same type structure.♻️ Suggested alignment
- qualities?: { quality: string, url: string }[]; + qualities?: { quality: string; url: string; frameRate?: number }[];src/components/stream/related-content/ClipDialog.tsx (1)
21-41: KeepclipQualitiesaligned with backend (includeframeRate).The backend now returns
frameRate; carrying it forward avoids losing metadata if the UI later wants FPS labels.🔧 Suggested tweak
- clipQualities?: { quality: string, url: string }[]; + clipQualities?: { quality: string; url: string; frameRate?: number }[];src/components/stream/related-content/index.tsx (1)
23-28: PreserveframeRateinclipQualitiesstate.Keeps parity with the backend payload and avoids dropping FPS metadata.
🔧 Suggested tweak
- const [clipQualities, setClipQualities] = useState<{ quality: string, url: string }[] | undefined>(undefined); + const [clipQualities, setClipQualities] = useState<{ quality: string; url: string; frameRate?: number }[] | undefined>(undefined);
| "rollup-plugin-visualizer": "^6.0.5", | ||
| "tailwindcss": "^4.1.18", | ||
| "typescript": "~5.6.3", | ||
| "vite": "^7.3.0", | ||
| "vite-plugin-compression": "^0.5.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
vite-plugin-compression 0.5.1 Vite 7 compatibility
💡 Result:
Short answer: The package's README and registry metadata state vite version ">= 2.0.0" (so v0.5.1 was not restricted to Vite 2/3/4), but there is no explicit mention of Vite 7 support — meaning it will often work but isn't guaranteed to be fully compatible with Vite 7's newer plugin/runtime changes. Test it in your project (or run a small build) and be prepared to upgrade/replace the plugin if you hit API breakage. [1][2][3]
Sources
- GitHub README (vite version: >=2.0.0). [1]
- npm package page for vite-plugin-compression (v0.5.1 metadata). [2]
- Libraries.io package entry (documentation mentioning vite >=2.0.0 / latest 0.5.1). [3]
Test vite-plugin-compression with Vite 7 before production use.
While vite-plugin-compression v0.5.1 declares support for Vite >=2.0.0, there is no explicit mention of Vite 7 support. The plugin may work but isn't guaranteed to be fully compatible with Vite 7's newer plugin/runtime changes. Test it in your build process and be prepared to upgrade or replace the plugin if API breakage occurs.
🤖 Prompt for AI Agents
In `@package.json` around lines 57 - 61, The dependency "vite-plugin-compression"
may not be proven compatible with Vite 7; verify compatibility by running a full
dev/build cycle with the current Vite version (the "vite" dependency) and watch
for plugin-related errors, then either upgrade "vite-plugin-compression" to a
Vite‑7‑compatible release or replace it with an alternative plugin if you
encounter API breakage; check and, if needed, update any usage sites (plugin
configuration in vite.config.* and any calls to compression() options) to match
the compatible plugin API before merging.
| /** | ||
| * Helper to ensure Kick date strings are properly formatted as UTC ISO strings | ||
| * Kick API sometimes returns "YYYY-MM-DD HH:MM:SS" (local/UTC ambiguous) or ISO without Z | ||
| * @returns Normalized ISO string with 'Z' suffix, or null if dateStr is missing/falsy | ||
| */ | ||
| export function normalizeKickDate(dateStr: string | null | undefined): string | null { | ||
| if (!dateStr) return null; | ||
|
|
||
| // Handle "YYYY-MM-DD HH:MM:SS" format (replace space with T, append Z) | ||
| if (dateStr.includes(' ') && !dateStr.includes('T')) { | ||
| return dateStr.replace(' ', 'T') + 'Z'; | ||
| } | ||
|
|
||
| // Handle ISO-like dates missing Timezone info (append Z) | ||
| if (dateStr.includes('T') && !dateStr.endsWith('Z') && !dateStr.includes('+')) { | ||
| return dateStr + 'Z'; | ||
| } | ||
|
|
||
| return dateStr; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle negative UTC offsets in timezone detection.
The check at line 74 misses negative timezone offsets (e.g., 2024-01-01T12:00:00-05:00). This could result in appending Z to an already valid ISO string with a negative offset, producing an invalid date.
🐛 Proposed fix
// Handle ISO-like dates missing Timezone info (append Z)
- if (dateStr.includes('T') && !dateStr.endsWith('Z') && !dateStr.includes('+')) {
+ if (dateStr.includes('T') && !dateStr.endsWith('Z') && !dateStr.includes('+') && !/[-+]\d{2}:\d{2}$/.test(dateStr)) {
return dateStr + 'Z';
}Alternatively, a simpler approach:
// Handle ISO-like dates missing Timezone info (append Z)
- if (dateStr.includes('T') && !dateStr.endsWith('Z') && !dateStr.includes('+')) {
+ // Check for Z, + offset, or - offset (but not the date separator -)
+ const hasTimezone = dateStr.endsWith('Z') || /[+-]\d{2}:\d{2}$/.test(dateStr);
+ if (dateStr.includes('T') && !hasTimezone) {
return dateStr + 'Z';
}📝 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.
| /** | |
| * Helper to ensure Kick date strings are properly formatted as UTC ISO strings | |
| * Kick API sometimes returns "YYYY-MM-DD HH:MM:SS" (local/UTC ambiguous) or ISO without Z | |
| * @returns Normalized ISO string with 'Z' suffix, or null if dateStr is missing/falsy | |
| */ | |
| export function normalizeKickDate(dateStr: string | null | undefined): string | null { | |
| if (!dateStr) return null; | |
| // Handle "YYYY-MM-DD HH:MM:SS" format (replace space with T, append Z) | |
| if (dateStr.includes(' ') && !dateStr.includes('T')) { | |
| return dateStr.replace(' ', 'T') + 'Z'; | |
| } | |
| // Handle ISO-like dates missing Timezone info (append Z) | |
| if (dateStr.includes('T') && !dateStr.endsWith('Z') && !dateStr.includes('+')) { | |
| return dateStr + 'Z'; | |
| } | |
| return dateStr; | |
| } | |
| /** | |
| * Helper to ensure Kick date strings are properly formatted as UTC ISO strings | |
| * Kick API sometimes returns "YYYY-MM-DD HH:MM:SS" (local/UTC ambiguous) or ISO without Z | |
| * `@returns` Normalized ISO string with 'Z' suffix, or null if dateStr is missing/falsy | |
| */ | |
| export function normalizeKickDate(dateStr: string | null | undefined): string | null { | |
| if (!dateStr) return null; | |
| // Handle "YYYY-MM-DD HH:MM:SS" format (replace space with T, append Z) | |
| if (dateStr.includes(' ') && !dateStr.includes('T')) { | |
| return dateStr.replace(' ', 'T') + 'Z'; | |
| } | |
| // Handle ISO-like dates missing Timezone info (append Z) | |
| // Check for Z, + offset, or - offset (but not the date separator -) | |
| const hasTimezone = dateStr.endsWith('Z') || /[+-]\d{2}:\d{2}$/.test(dateStr); | |
| if (dateStr.includes('T') && !hasTimezone) { | |
| return dateStr + 'Z'; | |
| } | |
| return dateStr; | |
| } |
🤖 Prompt for AI Agents
In `@src/backend/api/platforms/kick/kick-transformers.ts` around lines 60 - 79,
The timezone-detection logic in normalizeKickDate incorrectly treats ISO strings
with negative offsets as lacking timezone info; update the branch that currently
checks "dateStr.includes('T') && !dateStr.endsWith('Z') &&
!dateStr.includes('+')" to detect any timezone offset (both '+' and '-') instead
of only '+'. Concretely, in normalizeKickDate replace the simple '+' check with
a robust check for timezone suffixes (e.g., a regex that matches 'Z' or a +/−
offset like ±HH or ±HH:MM at the end) so strings like
"2024-01-01T12:00:00-05:00" are not appended with 'Z' and only truly
timezone-less strings get 'Z' appended.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Build & Performance
✏️ Tip: You can customize this high-level summary in your review settings.