Conversation
📝 WalkthroughWalkthroughThis PR comprehensively replaces the linear-player and youtube-music demo blocks with new audio-player and video-player blocks. It refactors the asset-loading hook to support normalized identities and dynamic source resolution, introduces a playback-source orchestration controller, and updates the home page, registry, and documentation accordingly. The changes enable flexible media playback configuration via asset resolution callbacks. ChangesAudio/Video Player Block Replacement with Asset Orchestration Refactoring
🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/www/registry/default/hooks/use-asset.ts (1)
453-497:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
preloadAsset()can publish stale managers out of order.
AbortControlleris created here but never used to cancel or reject older work, and there is no guard after anyawaitbefore writing back topreloadManagers. Two overlapping preloads for the same normalized asset can therefore resolve out of order, letting an older request destroy or replace a newer manager.As per coding guidelines, "Async functions must include abort/generation guards after each
awaitstatement."Also applies to: 500-515
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/registry/default/hooks/use-asset.ts` around lines 453 - 497, preloadAsset can publish stale shaka.media.PreloadManager instances because a new AbortController is created but previous controllers aren't aborted and there are no generation/abort guards after awaits; fix by: when starting a preload (in the preloadAsset flow that uses preloadDefault, resolveSourceValue, etc.) acquire or increment a per-asset generation/token or store the new AbortController in the preloadManagers map keyed by the normalized asset, abort the previous controller (if any) before starting, capture the current generation/token and localAbortController, pass localAbortController.signal into resolveSourceValue/preloadDefault, and after every await check that the stored controller/generation in preloadManagers still matches the local one before writing back manager; reference symbols: preloadDefault, abortController, preloadManagers, resolveSourceValue (and the enclosing preloadAsset flow) to implement these guards and cancellation.apps/www/content/docs/hooks/use-asset.mdx (1)
63-113:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the required hook doc sections (Store, Actions, Events).
The page is missing the required hook-structure sections for Store (with State table), Actions table, and Events table in the current content flow (Lines 63–113).
As per coding guidelines,apps/www/content/docs/hooks/**/*.mdx: "Hook documentation must include: Installation section, Feature registration code block, Store section with State table, Actions table, Events table, and AutoTypeTable component".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/content/docs/hooks/use-asset.mdx` around lines 63 - 113, The doc is missing the required hook-structure sections; add a "Store" section (with a State table), an "Actions" section (with an Actions table), and an "Events" section (with an Events table) into the content flow after the Options/Returns block (where lines 63–113 are). Use the AutoTypeTable component to reference the hook types from ./registry/default/hooks/use-asset.ts (for example AutoTypeTable path="./registry/default/hooks/use-asset.ts" name="UseAssetStore", name="UseAssetActions", and name="UseAssetEvents" or the actual exported type names in that file), and ensure each section has the required heading and table so the page conforms to the project's hook documentation structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/www/components/players/audio-player/demo-player.tsx`:
- Around line 26-32: The promise returned by
fetchAudioPlayerDemoPlaylist(playlistId, abortController.signal) is used without
a rejection handler; update the call in demo-player.tsx to append a .catch(...)
that ignores AbortError (or checks error.name === 'AbortError') and otherwise
logs or handles other errors and prevents unhandled promise rejections; keep the
existing then(...) behavior that checks abortController.signal.aborted and calls
setPlaylist(items).
In `@apps/www/content/docs/blocks/audio-player.mdx`:
- Around line 19-23: Update the import path for the audio player example to use
the limeplay component namespace: replace the current import of AudioPlayer and
AudioPlayerAsset from "`@/components/audio-player/components/media-player`" with
the equivalent "`@/components/limeplay/`..." path (e.g., import AudioPlayer, type
AudioPlayerAsset from "`@/components/limeplay/`...") and ensure any related hook
imports use "`@/hooks/limeplay/`..." so the example follows the docs guideline of
using `@/components/limeplay/`* and `@/hooks/limeplay/`*.
In `@apps/www/content/docs/blocks/video-player.mdx`:
- Around line 19-23: The import lines for VideoPlayer and type VideoPlayerAsset
currently use "`@/components/video-player/components/media-player`"; update them
to follow limeplay docs conventions by importing from the limeplay components
surface (e.g., change the module path to "`@/components/limeplay/`..." so
VideoPlayer and VideoPlayerAsset are imported from the limeplay component
module), ensuring documentation files use "`@/components/limeplay/`*" instead of
"`@/components/video-player/`*".
In `@apps/www/content/docs/hooks/use-playback-source.mdx`:
- Around line 17-56: The documentation for the new hook is missing the required
sections: add an "Installation" snippet, a Feature registration code block that
shows how to register PlaybackSourceController/UsePlaybackSourceOptions with the
feature registry, and add the Store (State) table, Actions table, and Events
table documenting the hook's state shape, public actions, and emitted events;
keep the existing AutoTypeTable (path
"./registry/default/hooks/use-playback-source.ts", name
"UsePlaybackSourceOptions") and ensure the tables reference the hook API symbols
PlaybackSourceController and UsePlaybackSourceOptions so readers can map
state/actions/events to the implementation.
In `@apps/www/registry/default/blocks/audio-player/components/audio-source.tsx`:
- Around line 130-139: The provider currently exposes placeholder fields (error,
isLoading, refetch) in the value object that overwrite the real asset state from
usePlaylistAsset, preventing pending/error transitions and making refetch a
no-op; fix this by either removing those placeholder fields from the value (so
the provider only exposes items) or by wiring the real controller state into the
provider (use the actual loading/error/refetch from usePlaylistAsset and pass
those through instead of the null/false/refetch stub); update the value
construction where refetch and value are defined (the refetch callback and the
value const) so they either omit error/isLoading/refetch or source them from the
real controller returned by usePlaylistAsset.
- Around line 106-127: The resolvedSource async callback can return a stale
source if the asset changes while fetchPlaybackUrl is in flight; after awaiting
fetchPlaybackUrl(asset.playbackUrls.primary, signal) add an abort/generation
guard (check signal.aborted or equivalent) and immediately throw or return to
bail if the operation was aborted before returning the resolved { config, src }
so resolvedSource does not produce a stale result.
In `@apps/www/registry/default/blocks/audio-player/components/button.tsx`:
- Around line 12-54: The Button component currently lets real <button> elements
inherit the browser default type="submit"; update the JSX in the
React.forwardRef implementation (Button, Comp, render, Slot) so that when Comp
resolves to "button" you ensure a default type of "button" if no type prop was
provided (e.g., compute a localType = props.type ?? "button" or conditionally
set type="button" on the element) before spreading {...props} and passing ref;
this guarantees native buttons rendered by Button won't submit forms
unexpectedly while still allowing callers to override type.
In `@apps/www/registry/default/blocks/audio-player/components/media-player.tsx`:
- Line 63: The Media element is hard-coded to autoPlay (<Media as="audio"
autoPlay />), causing unwanted immediate playback; update the exported component
(MediaPlayer / default export in media-player.tsx) to accept an optional prop
(e.g., autoPlay?: boolean = false) and pass that prop through to the Media
element instead of hard-coding it, ensuring the default is off and that source
changes only auto-play when the caller explicitly sets autoPlay to true.
- Around line 1-18: Add "use client" as the very first line of media-player.tsx
to make the module a client component (required because it passes function props
like getAssetId and resolveSource into client-side components
AudioSourceProvider and MediaProvider). Then add an optional autoPlay:boolean
prop to the AudioPlayerProps (default false) and update the Media JSX so it uses
that prop (e.g. <Media as="audio" autoPlay={autoPlay} ... />) instead of the
hard-coded <Media as="audio" autoPlay /> so callers can opt out of autoplay;
ensure any places that render the media-player component are updated or rely on
the new default to preserve existing behavior.
In
`@apps/www/registry/default/blocks/audio-player/components/playback-controls.tsx`:
- Around line 106-123: The effect that controls the delayed spinner uses
lastIntentRef.current but doesn't rerun when the playback intent/status changes,
so lingering timeouts can show the spinner incorrectly; update the useEffect
dependency array to include the reactive intent/status variable (e.g., status or
playbackStatus) so the effect re-evaluates when intent changes, and ensure
lastIntentRef is updated before the effect runs; keep the existing timeoutRef
and cleanup logic in the same useEffect (symbols: useEffect, lastIntentRef,
timeoutRef, isBuffering, delayMs, status/playbackStatus).
In `@apps/www/registry/default/blocks/audio-player/components/playlist.tsx`:
- Around line 181-185: The <img> in playlist.tsx currently uses
src={asset.poster ?? ""}, which produces an empty src when asset.poster is
missing; change this to render the <img> only when asset.poster is truthy (e.g.
conditionally return or inline conditional render using asset.poster) so no
<img> with an empty src is emitted; keep the same alt and className attributes
and src set to asset.poster when present.
In `@apps/www/registry/default/blocks/audio-player/components/track-info.tsx`:
- Around line 45-48: The subtitle currently includes a hard-coded year string
("• 2026") in the TrackInfo component; remove that literal and instead render a
real metadata field (e.g., use props.track.year, props.track.releaseYear,
metadata.year or similar) or omit the year suffix entirely when no year is
available—update the JSX around the genre variable in track-info.tsx to
conditionally append the actual year metadata if present rather than the
constant "2026".
In
`@apps/www/registry/default/blocks/audio-player/components/volume-group-control.tsx`:
- Around line 52-61: The mute/unmute SVG icons in the VolumeGroupControl
component are announced redundantly by assistive tech despite the button already
having an aria-label; update the rendering of VolumeMutedIcon and VolumeFullIcon
so they are treated as decorative (add aria-hidden="true" to the SVG elements or
wrap them in an element with aria-hidden) so screen readers do not announce the
icons — locate the conditional that renders VolumeMutedIcon / VolumeFullIcon
inside the AnimatePresence/motion span and ensure the icon components or their
root SVG elements include aria-hidden="true".
In `@apps/www/registry/default/blocks/video-player/components/media-player.tsx`:
- Around line 27-50: Remove the banned mediaRef prop from the public API: delete
the mediaRef field from the VideoPlayerProps interface and remove any
usage/forwarding of mediaRef elsewhere in this file (including the other
occurrence around the 88-95 range) so the component no longer accepts or exposes
mediaRef; update any prop destructuring, prop spreading, and types that
referenced mediaRef, and instead ensure callers access the underlying video
element via the supported hooks/components (e.g., useVideoPlayer hook or
internal media element refs) rather than passing mediaRef.
In `@apps/www/registry/default/blocks/video-player/components/playlist.tsx`:
- Around line 20-21: The component currently destructures currentItem,
isPreloaded, orderedItems, preloadAsset, and skipToId from useAsset(), which
subscribes the whole asset slice; replace that with granular selector
hooks—e.g., call the per-feature selector hook for each symbol (useAssetStore(s
=> s.currentItem), useAssetStore(s => s.isPreloaded), useAssetStore(s =>
s.orderedItems), and for actions useAssetStore(s => s.preloadAsset) and
useAssetStore(s => s.skipToId)) so the playlist only subscribes to the specific
fields it needs and avoids full-slice rerenders; update references in this file
to use the new selected values.
In `@apps/www/registry/default/hooks/use-asset.ts`:
- Around line 182-195: The store currently uses a single
installedOptions/optionsOwnerId slot which causes newer unmounts to wipe configs
still needed by other mounted useAsset() hooks; change the implementation to
maintain a per-owner map (e.g., optionsByOwner: Map<string,
UseAssetOptions<Asset>>) and update setOptions(ownerId, options) to store into
that map and set installedOptions/optionsOwnerId to that owner's value; change
clearOptions(ownerId) to remove only that owner's entry and, if the removed
owner was the active optionsOwnerId, select and restore another still-mounted
owner's options (e.g., most-recently-set or remaining map entry) into
installedOptions/optionsOwnerId so remaining hooks keep their
getAssetId/resolveSource/error handlers; update any other places referencing
installedOptions/optionsOwnerId (including preloadNext, loadAsset, and any logic
around retryCount/previousError) to read from the per-owner map or
installedOptions as restored.
In `@apps/www/registry/default/hooks/use-playback-source.ts`:
- Around line 50-60: loadedSourceKeyRef persists across player teardown so when
the player instance is recreated (via usePlayerStore -> player) the unchanged
resolvedSourceKey can short-circuit loadPlaylist; detect player changes and
reset loadedSourceKeyRef.current to null when a new player instance appears (for
example inside the effect that depends on player or where you initialize
resolveSource/loadPlaylist) so that loadPlaylist is allowed to run for the new
player; refer to loadedSourceKeyRef, player (from usePlayerStore),
resolvedSourceKey and loadPlaylist to locate where to add the reset.
In `@apps/www/registry/pro`:
- Line 1: The submodule update for apps/www/registry/pro removed TypeScript
sources required by the registry-dev flow; verify that the checked-out commit
53d1af0f... actually contains the expected .ts/.tsx files and if it does not,
either update the submodule to a commit that includes those sources or modify
the consumer in apps/www/scripts/registry-dev.mts to handle the new tree (remove
or adapt the code that assumes recursive .ts/.tsx files). Locate references to
the registry submodule usage in apps/www/scripts/registry-dev.mts (the code that
previously iterated/checked for .ts/.tsx under apps/www/registry/pro) and update
it to either read the new file layout or fail fast with a clear error message so
downstream flows aren't broken.
---
Outside diff comments:
In `@apps/www/content/docs/hooks/use-asset.mdx`:
- Around line 63-113: The doc is missing the required hook-structure sections;
add a "Store" section (with a State table), an "Actions" section (with an
Actions table), and an "Events" section (with an Events table) into the content
flow after the Options/Returns block (where lines 63–113 are). Use the
AutoTypeTable component to reference the hook types from
./registry/default/hooks/use-asset.ts (for example AutoTypeTable
path="./registry/default/hooks/use-asset.ts" name="UseAssetStore",
name="UseAssetActions", and name="UseAssetEvents" or the actual exported type
names in that file), and ensure each section has the required heading and table
so the page conforms to the project's hook documentation structure.
In `@apps/www/registry/default/hooks/use-asset.ts`:
- Around line 453-497: preloadAsset can publish stale shaka.media.PreloadManager
instances because a new AbortController is created but previous controllers
aren't aborted and there are no generation/abort guards after awaits; fix by:
when starting a preload (in the preloadAsset flow that uses preloadDefault,
resolveSourceValue, etc.) acquire or increment a per-asset generation/token or
store the new AbortController in the preloadManagers map keyed by the normalized
asset, abort the previous controller (if any) before starting, capture the
current generation/token and localAbortController, pass
localAbortController.signal into resolveSourceValue/preloadDefault, and after
every await check that the stored controller/generation in preloadManagers still
matches the local one before writing back manager; reference symbols:
preloadDefault, abortController, preloadManagers, resolveSourceValue (and the
enclosing preloadAsset flow) to implement these guards and cancellation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7d589ede-72fe-46bd-b116-e6584d854876
⛔ Files ignored due to path filters (4)
apps/www/next-env.d.tsis excluded by none and included by noneapps/www/scripts/test-registry-install.tsis excluded by none and included by noneapps/www/scripts/validate-registries.tsis excluded by none and included by noneapps/www/vercel.jsonis excluded by none and included by none
📒 Files selected for processing (69)
apps/www/app/(home)/page.tsxapps/www/components/blocks/block-showcase.tsxapps/www/components/blocks/info-pane.tsxapps/www/components/blocks/preview-pane.tsxapps/www/components/hero-buttons.tsxapps/www/components/mdx-components.tsxapps/www/components/players/audio-player/demo-player.tsxapps/www/components/players/audio-player/demo.tsapps/www/components/players/audio-player/hover-player.tsxapps/www/components/players/video-player/demo-assets.tsapps/www/components/players/video-player/player-container.tsxapps/www/content/docs/blocks/audio-player.mdxapps/www/content/docs/blocks/linear-player.mdxapps/www/content/docs/blocks/video-player.mdxapps/www/content/docs/blocks/youtube-music.mdxapps/www/content/docs/hooks/meta.jsonapps/www/content/docs/hooks/use-asset.mdxapps/www/content/docs/hooks/use-playback-source.mdxapps/www/content/docs/quick-start.mdxapps/www/registry/collection/registry-blocks.tsapps/www/registry/collection/registry-hooks.tsapps/www/registry/default/blocks/audio-player/audio-player.module.cssapps/www/registry/default/blocks/audio-player/components/action-controls.tsxapps/www/registry/default/blocks/audio-player/components/audio-source.tsxapps/www/registry/default/blocks/audio-player/components/button.tsxapps/www/registry/default/blocks/audio-player/components/controls.tsxapps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsxapps/www/registry/default/blocks/audio-player/components/icons.tsxapps/www/registry/default/blocks/audio-player/components/media-player.tsxapps/www/registry/default/blocks/audio-player/components/playback-controls.tsxapps/www/registry/default/blocks/audio-player/components/playback-mode-controls.tsxapps/www/registry/default/blocks/audio-player/components/playlist.tsxapps/www/registry/default/blocks/audio-player/components/track-info.tsxapps/www/registry/default/blocks/audio-player/components/volume-group-control.tsxapps/www/registry/default/blocks/audio-player/hooks/use-playlist-asset.tsapps/www/registry/default/blocks/audio-player/lib/media-kit.tsapps/www/registry/default/blocks/basic-player/components/media-element.tsxapps/www/registry/default/blocks/basic-player/components/media-player.tsxapps/www/registry/default/blocks/basic-player/components/playback-state-control.tsxapps/www/registry/default/blocks/basic-player/lib/media-kit.tsapps/www/registry/default/blocks/basic-player/page.tsxapps/www/registry/default/blocks/linear-player/components/media-player.tsxapps/www/registry/default/blocks/linear-player/page.tsxapps/www/registry/default/blocks/video-player/components/bottom-controls.tsxapps/www/registry/default/blocks/video-player/components/button.tsxapps/www/registry/default/blocks/video-player/components/captions-state-control.tsxapps/www/registry/default/blocks/video-player/components/media-player.tsxapps/www/registry/default/blocks/video-player/components/pip-control.tsxapps/www/registry/default/blocks/video-player/components/playback-rate-control.tsxapps/www/registry/default/blocks/video-player/components/playback-state-control.tsxapps/www/registry/default/blocks/video-player/components/playlist.tsxapps/www/registry/default/blocks/video-player/components/timeline-slider-control.tsxapps/www/registry/default/blocks/video-player/components/volume-group-control.tsxapps/www/registry/default/blocks/video-player/components/volume-slider-control.tsxapps/www/registry/default/blocks/video-player/components/volume-state-control.tsxapps/www/registry/default/blocks/video-player/lib/media-kit.tsapps/www/registry/default/blocks/video-player/page.tsxapps/www/registry/default/blocks/video-player/ui/toggle.tsxapps/www/registry/default/examples/picture-in-picture-control-demo.tsxapps/www/registry/default/hooks/use-asset.tsapps/www/registry/default/hooks/use-media.tsapps/www/registry/default/hooks/use-playback-source.tsapps/www/registry/default/hooks/use-player.tsapps/www/registry/default/hooks/use-playlist.tsapps/www/registry/default/ui/limeplay-logo.tsxapps/www/registry/default/ui/media-provider.tsxapps/www/registry/default/ui/media.tsxapps/www/registry/proprompts/ide.md
💤 Files with no reviewable changes (9)
- apps/www/registry/default/blocks/basic-player/components/playback-state-control.tsx
- apps/www/content/docs/blocks/youtube-music.mdx
- apps/www/content/docs/blocks/linear-player.mdx
- apps/www/registry/default/blocks/basic-player/page.tsx
- apps/www/registry/default/blocks/linear-player/page.tsx
- apps/www/registry/default/blocks/basic-player/components/media-player.tsx
- apps/www/registry/default/blocks/basic-player/components/media-element.tsx
- apps/www/registry/default/blocks/basic-player/lib/media-kit.ts
- apps/www/registry/default/blocks/linear-player/components/media-player.tsx
Comment on lines
+26
to
+32
| void fetchAudioPlayerDemoPlaylist(playlistId, abortController.signal).then( | ||
| (items) => { | ||
| if (!abortController.signal.aborted) { | ||
| setPlaylist(items) | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
Handle rejected playlist fetch to avoid unhandled promise rejections.
Line 26 starts a promise chain without a rejection handler. Add .catch(...) (ignore AbortError, handle others) so failures don’t surface as unhandled rejections.
Suggested fix
- void fetchAudioPlayerDemoPlaylist(playlistId, abortController.signal).then(
- (items) => {
- if (!abortController.signal.aborted) {
- setPlaylist(items)
- }
- }
- )
+ void fetchAudioPlayerDemoPlaylist(playlistId, abortController.signal)
+ .then((items) => {
+ if (!abortController.signal.aborted) {
+ setPlaylist(items)
+ }
+ })
+ .catch((error) => {
+ if ((error as { name?: string })?.name === "AbortError") return
+ // Optionally report/log and keep previous playlist
+ })📝 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
| void fetchAudioPlayerDemoPlaylist(playlistId, abortController.signal).then( | |
| (items) => { | |
| if (!abortController.signal.aborted) { | |
| setPlaylist(items) | |
| } | |
| } | |
| ) | |
| void fetchAudioPlayerDemoPlaylist(playlistId, abortController.signal) | |
| .then((items) => { | |
| if (!abortController.signal.aborted) { | |
| setPlaylist(items) | |
| } | |
| }) | |
| .catch((error) => { | |
| if ((error as { name?: string })?.name === "AbortError") return | |
| // Optionally report/log and keep previous playlist | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/components/players/audio-player/demo-player.tsx` around lines 26 -
32, The promise returned by fetchAudioPlayerDemoPlaylist(playlistId,
abortController.signal) is used without a rejection handler; update the call in
demo-player.tsx to append a .catch(...) that ignores AbortError (or checks
error.name === 'AbortError') and otherwise logs or handles other errors and
prevents unhandled promise rejections; keep the existing then(...) behavior that
checks abortController.signal.aborted and calls setPlaylist(items).
Comment on lines
+19
to
+23
| import { | ||
| AudioPlayer, | ||
| type AudioPlayerAsset, | ||
| } from "@/components/audio-player/components/media-player" | ||
|
|
There was a problem hiding this comment.
Use limeplay import paths in docs examples.
Line 22 imports from @/components/audio-player/...; docs examples should use @/components/limeplay/* (and hooks from @/hooks/limeplay/*) for consistency with install targets.
As per coding guidelines, {apps/www/content/docs/**/*.{md,mdx},...}: "In documentation files, use imports from @/hooks/limeplay/* and @/components/limeplay/*—NEVER use @/registry/... imports".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/content/docs/blocks/audio-player.mdx` around lines 19 - 23, Update
the import path for the audio player example to use the limeplay component
namespace: replace the current import of AudioPlayer and AudioPlayerAsset from
"`@/components/audio-player/components/media-player`" with the equivalent
"`@/components/limeplay/`..." path (e.g., import AudioPlayer, type
AudioPlayerAsset from "`@/components/limeplay/`...") and ensure any related hook
imports use "`@/hooks/limeplay/`..." so the example follows the docs guideline of
using `@/components/limeplay/`* and `@/hooks/limeplay/`*.
Comment on lines
+19
to
+23
| import { | ||
| VideoPlayer, | ||
| type VideoPlayerAsset, | ||
| } from "@/components/video-player/components/media-player" | ||
|
|
There was a problem hiding this comment.
Align usage imports with limeplay docs conventions.
Line 22 currently uses @/components/video-player/...; this should be documented with @/components/limeplay/* paths to match the docs conventions and generated install surface.
As per coding guidelines, {apps/www/content/docs/**/*.{md,mdx},...}: "In documentation files, use imports from @/hooks/limeplay/* and @/components/limeplay/*—NEVER use @/registry/... imports".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/content/docs/blocks/video-player.mdx` around lines 19 - 23, The
import lines for VideoPlayer and type VideoPlayerAsset currently use
"`@/components/video-player/components/media-player`"; update them to follow
limeplay docs conventions by importing from the limeplay components surface
(e.g., change the module path to "`@/components/limeplay/`..." so VideoPlayer and
VideoPlayerAsset are imported from the limeplay component module), ensuring
documentation files use "`@/components/limeplay/`*" instead of
"`@/components/video-player/`*".
Comment on lines
+17
to
+56
| ## Usage | ||
|
|
||
| Use `PlaybackSourceController` inside a block to load exactly one source model: `asset`, `playlist`, or `mediaSrc`. | ||
|
|
||
| ```tsx | ||
| import { PlaybackSourceController } from "@/hooks/limeplay/use-playback-source" | ||
|
|
||
| export function SourceController({ src }: { src?: string }) { | ||
| return <PlaybackSourceController mediaSrc={src} /> | ||
| } | ||
| ``` | ||
|
|
||
| For typed assets, pass `asset`, `playlist`, `getAssetId`, and `resolveSource` through to `useAsset`. | ||
|
|
||
| ```tsx | ||
| import type { Asset } from "@/hooks/limeplay/use-asset" | ||
| import { PlaybackSourceController } from "@/hooks/limeplay/use-playback-source" | ||
|
|
||
| interface VideoAsset extends Asset { | ||
| playbackUrl: string | ||
| slug: string | ||
| } | ||
|
|
||
| export function VideoSource({ playlist }: { playlist: VideoAsset[] }) { | ||
| return ( | ||
| <PlaybackSourceController | ||
| getAssetId={(asset) => asset.slug} | ||
| playlist={playlist} | ||
| resolveSource={({ asset }) => asset.playbackUrl} | ||
| /> | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| ## API Reference | ||
|
|
||
| <AutoTypeTable | ||
| path="./registry/default/hooks/use-playback-source.ts" | ||
| name="UsePlaybackSourceOptions" | ||
| /> |
There was a problem hiding this comment.
Complete the required hook documentation template sections.
This new hook page is missing the mandatory feature registration code block plus Store (State table), Actions table, and Events table sections.
As per coding guidelines, apps/www/content/docs/hooks/**/*.mdx: "Hook documentation must include: Installation section, Feature registration code block, Store section with State table, Actions table, Events table, and AutoTypeTable component".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/content/docs/hooks/use-playback-source.mdx` around lines 17 - 56,
The documentation for the new hook is missing the required sections: add an
"Installation" snippet, a Feature registration code block that shows how to
register PlaybackSourceController/UsePlaybackSourceOptions with the feature
registry, and add the Store (State) table, Actions table, and Events table
documenting the hook's state shape, public actions, and emitted events; keep the
existing AutoTypeTable (path "./registry/default/hooks/use-playback-source.ts",
name "UsePlaybackSourceOptions") and ensure the tables reference the hook API
symbols PlaybackSourceController and UsePlaybackSourceOptions so readers can map
state/actions/events to the implementation.
Comment on lines
+106
to
+127
| const resolvedSource = React.useCallback<ResolveSource<AudioPlayerAsset>>( | ||
| async (context) => { | ||
| if (resolveSource) return resolveSource(context) | ||
|
|
||
| const { asset, signal } = context | ||
| if (asset.src) { | ||
| return { | ||
| config: asset.config, | ||
| src: asset.src, | ||
| } | ||
| } | ||
|
|
||
| if (!asset.playbackUrls?.primary) { | ||
| throw new Error("AudioPlayerAsset requires src or playbackUrls.primary") | ||
| } | ||
|
|
||
| const src = await fetchPlaybackUrl(asset.playbackUrls.primary, signal) | ||
| return { | ||
| config: asset.config, | ||
| src, | ||
| } | ||
| }, |
There was a problem hiding this comment.
Add an abort guard after the playback URL fetch.
If the active asset changes while fetchPlaybackUrl() is in flight, this callback can still return a stale source after cancellation. Bail immediately after the await before returning the resolved URL.
Suggested fix
const resolvedSource = React.useCallback<ResolveSource<AudioPlayerAsset>>(
async (context) => {
if (resolveSource) return resolveSource(context)
const { asset, signal } = context
if (asset.src) {
return {
config: asset.config,
src: asset.src,
}
}
if (!asset.playbackUrls?.primary) {
throw new Error("AudioPlayerAsset requires src or playbackUrls.primary")
}
const src = await fetchPlaybackUrl(asset.playbackUrls.primary, signal)
+ if (signal?.aborted) {
+ throw new DOMException("Aborted", "AbortError")
+ }
return {
config: asset.config,
src,
}
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/registry/default/blocks/audio-player/components/audio-source.tsx`
around lines 106 - 127, The resolvedSource async callback can return a stale
source if the asset changes while fetchPlaybackUrl is in flight; after awaiting
fetchPlaybackUrl(asset.playbackUrls.primary, signal) add an abort/generation
guard (check signal.aborted or equivalent) and immediately throw or return to
bail if the operation was aborted before returning the resolved { config, src }
so resolvedSource does not produce a stale result.
Comment on lines
+27
to
+50
| export interface VideoPlayerProps { | ||
| asset?: VideoPlayerAsset | ||
| assetOptions?: Omit< | ||
| UseAssetOptions<VideoPlayerAsset>, | ||
| "getAssetId" | "resolveSource" | ||
| > | ||
| children?: React.ReactNode | ||
| className?: string | ||
| debug?: boolean | ||
| getAssetId?: GetAssetId<VideoPlayerAsset> | ||
| initialIndex?: number | ||
| /** | ||
| * Props to pass to the underlying video element. | ||
| */ | ||
| mediaProps?: Omit< | ||
| React.VideoHTMLAttributes<HTMLVideoElement>, | ||
| "as" | "className" | ||
| > | ||
| /** | ||
| * Ref to the underlying video element. | ||
| */ | ||
| mediaRef?: React.Ref<HTMLVideoElement> | ||
| playlist?: VideoPlayerAsset[] | ||
| resolveSource?: ResolveSource<VideoPlayerAsset> |
There was a problem hiding this comment.
Remove the banned mediaRef prop from this public API.
This reintroduces mediaRef on a brand-new component surface, which will push downstream usage back onto an explicitly deleted API. Please drop the prop and route media-element access through the supported hooks/components instead. As per coding guidelines, Do NOT use deleted/deprecated APIs: ... mediaRef ....
Also applies to: 88-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/registry/default/blocks/video-player/components/media-player.tsx`
around lines 27 - 50, Remove the banned mediaRef prop from the public API:
delete the mediaRef field from the VideoPlayerProps interface and remove any
usage/forwarding of mediaRef elsewhere in this file (including the other
occurrence around the 88-95 range) so the component no longer accepts or exposes
mediaRef; update any prop destructuring, prop spreading, and types that
referenced mediaRef, and instead ensure callers access the underlying video
element via the supported hooks/components (e.g., useVideoPlayer hook or
internal media element refs) rather than passing mediaRef.
Comment on lines
+20
to
+21
| const { currentItem, isPreloaded, orderedItems, preloadAsset, skipToId } = | ||
| useAsset<VideoPlayerAsset>() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Switch this playlist off the whole-slice useAsset() subscription.
Destructuring multiple fields from useAsset() subscribes the component to the entire asset slice, so unrelated asset mutations will rerender the menu. Please use granular selector-based access here instead. As per coding guidelines, Always use granular per-feature selectors like useXxxStore(s => s.field) for state access—never access entire slices.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/registry/default/blocks/video-player/components/playlist.tsx` around
lines 20 - 21, The component currently destructures currentItem, isPreloaded,
orderedItems, preloadAsset, and skipToId from useAsset(), which subscribes the
whole asset slice; replace that with granular selector hooks—e.g., call the
per-feature selector hook for each symbol (useAssetStore(s => s.currentItem),
useAssetStore(s => s.isPreloaded), useAssetStore(s => s.orderedItems), and for
actions useAssetStore(s => s.preloadAsset) and useAssetStore(s => s.skipToId))
so the playlist only subscribes to the specific fields it needs and avoids
full-slice rerenders; update references in this file to use the new selected
values.
Comment on lines
+182
to
195
| clearOptions: (ownerId: string) => void | ||
| installedOptions?: UseAssetOptions<Asset> | ||
| isFirstLoad: boolean | ||
| loadAbortController: AbortController | null | ||
| loadAsset: (asset: Asset, startTime?: number) => Promise<boolean> | ||
| loadGeneration: number | ||
| loadPlaylist: (assets: Asset[], startIndex?: number) => void | ||
| optionsOwnerId: null | string | ||
| preloadAsset: (asset: Asset) => Promise<void> | ||
| preloadNext: () => Promise<void> | ||
| previousError: unknown | ||
| retryCount: number | ||
| setOptions: (options?: UseAssetOptions<Asset>) => void | ||
| setOptions: (options: UseAssetOptions<Asset>, ownerId: string) => void | ||
| } |
There was a problem hiding this comment.
Single-owner option storage drops still-mounted useAsset() configs.
This store only remembers one installedOptions/optionsOwnerId. If two configured useAsset() hooks are mounted in the same provider, unmounting the newer one clears the active options even while the older hook is still mounted, so later loads lose getAssetId, resolveSource, and error handlers. This needs per-owner storage with restore-on-unmount semantics, not a single slot.
Also applies to: 214-221, 537-541, 568-581
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/registry/default/hooks/use-asset.ts` around lines 182 - 195, The
store currently uses a single installedOptions/optionsOwnerId slot which causes
newer unmounts to wipe configs still needed by other mounted useAsset() hooks;
change the implementation to maintain a per-owner map (e.g., optionsByOwner:
Map<string, UseAssetOptions<Asset>>) and update setOptions(ownerId, options) to
store into that map and set installedOptions/optionsOwnerId to that owner's
value; change clearOptions(ownerId) to remove only that owner's entry and, if
the removed owner was the active optionsOwnerId, select and restore another
still-mounted owner's options (e.g., most-recently-set or remaining map entry)
into installedOptions/optionsOwnerId so remaining hooks keep their
getAssetId/resolveSource/error handlers; update any other places referencing
installedOptions/optionsOwnerId (including preloadNext, loadAsset, and any logic
around retryCount/previousError) to read from the per-owner map or
installedOptions as restored.
Comment on lines
+50
to
+60
| const player = usePlayerStore((state) => state.instance) | ||
| const installedOptions = useMemo<UseAssetOptions<TAsset>>( | ||
| () => ({ | ||
| ...assetOptions, | ||
| getAssetId, | ||
| resolveSource, | ||
| }), | ||
| [assetOptions, getAssetId, resolveSource] | ||
| ) | ||
| const { loadPlaylist } = useAsset<TAsset>(installedOptions) | ||
| const loadedSourceKeyRef = useRef<null | string>(null) |
There was a problem hiding this comment.
Reset the dedupe key when the player instance is recreated.
loadedSourceKeyRef survives player teardown, so when debug or the media element recreates Shaka, the unchanged resolvedSourceKey short-circuits loadPlaylist() and the new player never reloads the active source.
Suggested fix
const { loadPlaylist } = useAsset<TAsset>(installedOptions)
const loadedSourceKeyRef = useRef<null | string>(null)
+
+ useEffect(() => {
+ loadedSourceKeyRef.current = null
+ }, [player])
const assets = useMemo(() => {Also applies to: 88-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/registry/default/hooks/use-playback-source.ts` around lines 50 - 60,
loadedSourceKeyRef persists across player teardown so when the player instance
is recreated (via usePlayerStore -> player) the unchanged resolvedSourceKey can
short-circuit loadPlaylist; detect player changes and reset
loadedSourceKeyRef.current to null when a new player instance appears (for
example inside the effect that depends on player or where you initialize
resolveSource/loadPlaylist) so that loadPlaylist is allowed to run for the new
player; refer to loadedSourceKeyRef, player (from usePlayerStore),
resolvedSourceKey and loadPlaylist to locate where to add the reset.
| @@ -1 +1 @@ | |||
| Subproject commit 7c47ec191091bcaa5a02df8f2eab484f0f85e1c6 | |||
| Subproject commit 53d1af0f2fe113749cda35ca9133ae0ee5e68f8e | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
old_sha="7c47ec191091bcaa5a02df8f2eab484f0f85e1c6"
new_sha="53d1af0f2fe113749cda35ca9133ae0ee5e68f8e"
echo "Current gitlink:"
git ls-tree HEAD apps/www/registry/pro
submodule_key="$(
git config -f .gitmodules --get-regexp '^submodule\..*\.path$' \
| awk '$2=="apps/www/registry/pro" { print $1 }' \
| sed 's/\.path$//'
)"
if [ -z "${submodule_key:-}" ]; then
echo "Could not find apps/www/registry/pro in .gitmodules" >&2
exit 1
fi
submodule_url="$(git config -f .gitmodules --get "${submodule_key}.url")"
owner_repo="$(printf '%s' "$submodule_url" | sed -E 's#(git@github.com:|https://github.com/)##; s#\.git$##')"
echo
echo "Submodule metadata:"
printf ' key: %s\n url: %s\n' "$submodule_key" "$submodule_url"
echo
echo "Verify both revisions exist in the remote:"
git ls-remote "$submodule_url" "$old_sha" "$new_sha"
echo
echo "Changed files between the two submodule revisions:"
curl -fsSL "https://api.github.com/repos/${owner_repo}/compare/${old_sha}...${new_sha}" \
| jq -r '.files[]?.filename'
echo
echo "Count TS/TSX files in the new submodule tree:"
curl -fsSL "https://api.github.com/repos/${owner_repo}/git/trees/${new_sha}?recursive=1" \
| jq -r '[.tree[]? | select(.type == "blob" and (.path | endswith(".ts") or endswith(".tsx")))] | length'Repository: WINOFFRG/limeplay
Length of output: 484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
old_sha="7c47ec191091bcaa5a02df8f2eab484f0f85e1c6"
new_sha="53d1af0f2fe113749cda35ca9133ae0ee5e68f8e"
submodule_url="$(git config -f .gitmodules --get 'submodule.apps/www/registry/pro.url')"
owner_repo="$(printf '%s' "$submodule_url" | sed -E 's#(git@github.com:|https://github.com/)##; s#\.git$##')"
echo "Submodule URL: $submodule_url"
echo "owner/repo: $owner_repo"
echo
echo "Changed files between the two submodule revisions:"
curl -fsSL "https://api.github.com/repos/${owner_repo}/compare/${old_sha}...${new_sha}" \
| jq -r '.files[]?.filename'
echo
echo "Count TS/TSX files in the new submodule tree:"
count="$(curl -fsSL "https://api.github.com/repos/${owner_repo}/git/trees/${new_sha}?recursive=1" \
| jq -r '[.tree[]? | select(.type == "blob" and (.path | endswith(".ts") or endswith(".tsx")))] | length')"
echo "TS/TSX blob count: $count"
if [ "$count" -le 0 ]; then
echo "Expected > 0 TS/TSX blobs to satisfy `apps/www/scripts/registry-dev.mts`." >&2
exit 1
fiRepository: WINOFFRG/limeplay
Length of output: 271
🏁 Script executed:
#!/bin/bash
set -euo pipefail
new_sha="53d1af0f2fe113749cda35ca9133ae0ee5e68f8e"
echo "Submodule status:"
git submodule status || true
echo
echo "Is the submodule directory present in the working tree?"
if [ -d "apps/www/registry/pro" ]; then
echo "YES"
echo "Count .ts/.tsx files in current submodule checkout:"
find apps/www/registry/pro -type f \( -name '*.ts' -o -name '*.tsx' \) | wc -l
echo
if [ -d "apps/www/registry/pro/.git" ] || [ -d ".git/modules/apps/www/registry/pro" ]; then
echo "Current submodule HEAD:"
(cd apps/www/registry/pro && git rev-parse HEAD) || true
else
echo "Submodule git metadata not present; cannot confirm local HEAD."
fi
else
echo "NO (submodule not checked out locally)."
fiRepository: WINOFFRG/limeplay
Length of output: 322
Submodule bump likely breaks registry/pro TypeScript contract (Line 1)
apps/www/registry/pro is bumped to 53d1af0f2fe113749cda35ca9133ae0ee5e68f8e, but the checked-out apps/www/registry/pro directory contains 0 *.ts/*.tsx files (recursive). If apps/www/scripts/registry-dev.mts:311-323 expects those files, this change will break the downstream registry-dev flow—please confirm the new submodule tree still includes the expected TypeScript sources (or adjust the consumer).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/registry/pro` at line 1, The submodule update for
apps/www/registry/pro removed TypeScript sources required by the registry-dev
flow; verify that the checked-out commit 53d1af0f... actually contains the
expected .ts/.tsx files and if it does not, either update the submodule to a
commit that includes those sources or modify the consumer in
apps/www/scripts/registry-dev.mts to handle the new tree (remove or adapt the
code that assumes recursive .ts/.tsx files). Locate references to the registry
submodule usage in apps/www/scripts/registry-dev.mts (the code that previously
iterated/checked for .ts/.tsx under apps/www/registry/pro) and update it to
either read the new file layout or fail fast with a clear error message so
downstream flows aren't broken.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Refactoring