diff --git a/next-cloudinary/src/components/CldVideoPlayer/CldVideoPlayer.tsx b/next-cloudinary/src/components/CldVideoPlayer/CldVideoPlayer.tsx index b327aa43..147dba5e 100644 --- a/next-cloudinary/src/components/CldVideoPlayer/CldVideoPlayer.tsx +++ b/next-cloudinary/src/components/CldVideoPlayer/CldVideoPlayer.tsx @@ -1,86 +1,84 @@ -import React, {useRef, MutableRefObject, useEffect, useId} from 'react'; +/** + * @issue https://github.com/cloudinary-community/next-cloudinary/issues/575 + * The original component accepted a custom `playerRef` prop. This ref was populated + * asynchronously after an external script loaded, making it impossible for parent + * components to reliably access the player instance on mount (e.g., in a useEffect). + * + * @solution + * This refactor upgrades the component to use standard React ref forwarding. + * 1. It is wrapped in `React.forwardRef` to accept a standard `ref` attribute. + * 2. It uses a `useState` hook (`player`) to trigger a re-render when the async + * player instance is created. + * 3. `useImperativeHandle` is used to expose the player instance on the parent's + * ref only after it has been successfully created, ensuring the ref is + * populated correctly and reliably. + */ +import React, { useState, useRef, MutableRefObject, useEffect, useId, forwardRef, useImperativeHandle } from 'react'; import Script from 'next/script'; import Head from 'next/head'; import { CloudinaryVideoPlayer } from '@cloudinary-util/types'; import { getVideoPlayerOptions } from '@cloudinary-util/url-loader'; import { CldVideoPlayerProps } from './CldVideoPlayer.types'; - import { getCloudinaryConfig } from "../../lib/cloudinary"; let playerInstances: string[] = []; - const PLAYER_VERSION = '1.11.1'; -const CldVideoPlayer = (props: CldVideoPlayerProps) => { +const CldVideoPlayer = forwardRef((props, ref) => { + + const [player, setPlayer] = useState(null); const { - className, - config, - height, - id, - onDataLoad, - onError, - onMetadataLoad, - onPause, - onPlay, - onEnded, - width, + className, config, height, id, onDataLoad, onError, onMetadataLoad, onPause, onPlay, onEnded, width, + // The old `playerRef` prop is destructured and ignored to favor the new standard `ref` system. + playerRef: unusedPlayerRef, } = props; const uniqueId = useId(); - const cloudinaryConfig = getCloudinaryConfig(config); const playerOptions = getVideoPlayerOptions(props, cloudinaryConfig); const { publicId } = playerOptions; - if ( typeof publicId === 'undefined' ) { + + if (typeof publicId === 'undefined') { throw new Error('Video Player requires a Public ID or Cloudinary URL - please specify a src prop'); } - // Setup the refs and allow for the caller to pass through their - // own ref instance - const cloudinaryRef = useRef(); const defaultVideoRef = useRef() as MutableRefObject; const videoRef = props.videoRef || defaultVideoRef; - const defaultPlayerRef = useRef()as MutableRefObject; - const playerRef = props.playerRef || defaultPlayerRef; + + const playerRef = useRef(null); + + // Expose the player state via the forwarded ref. The dependency array ensures this + // updates the parent's ref only when the player instance has been created. + useImperativeHandle(ref, () => player as CloudinaryVideoPlayer, [player]); const playerId = id || `player-${uniqueId.replace(/:/g, '')}`; let playerClassName = 'cld-video-player cld-fluid'; - if ( className ) { + if (className) { playerClassName = `${playerClassName} ${className}`; } - // Check if the same id is being used for multiple video instances. - const checkForMultipleInstance = playerInstances.filter((id) => id === playerId).length > 1 + const checkForMultipleInstance = playerInstances.filter((id) => id === playerId).length > 1; if (checkForMultipleInstance) { - console.warn(`Multiple instances of the same video detected on the - page which may cause some features to not work. - Try adding a unique id to each player.`) + console.warn(`Multiple instances of the same video detected on the page which may cause some features to not work. Try adding a unique id to each player.`); } else { - playerInstances.push(playerId) + playerInstances.push(playerId); } - const events: Record = { - error: onError, - loadeddata: onDataLoad, - loadedmetadata: onMetadataLoad, - pause: onPause, - play: onPlay, - ended: onEnded + const events: Record = { + error: onError, loadeddata: onDataLoad, loadedmetadata: onMetadataLoad, pause: onPause, play: onPlay, ended: onEnded }; /** * handleEvent * @description Event handler for all player events */ - function handleEvent(event: { type: 'string' }) { const activeEvent = events[event.type]; - - if ( typeof activeEvent === 'function' ) { + if (typeof activeEvent === 'function') { activeEvent(getPlayerRefs()); } } @@ -89,14 +87,18 @@ const CldVideoPlayer = (props: CldVideoPlayerProps) => { * handleOnLoad * @description Stores the Cloudinary window instance to a ref when the widget script loads */ - function handleOnLoad() { - if ( 'cloudinary' in window ) { + if ('cloudinary' in window) { cloudinaryRef.current = window.cloudinary; - playerRef.current = cloudinaryRef.current.videoPlayer(videoRef.current, playerOptions); + const newPlayer = cloudinaryRef.current.videoPlayer(videoRef.current, playerOptions); + + playerRef.current = newPlayer; + + // Set the player state to trigger a re-render, which in turn updates the forwarded ref. + setPlayer(newPlayer); Object.keys(events).forEach((key) => { - if ( typeof events[key] === 'function' ) { + if (typeof events[key] === 'function') { playerRef.current?.on(key, handleEvent); } }); @@ -104,22 +106,17 @@ const CldVideoPlayer = (props: CldVideoPlayerProps) => { } useEffect(() => { - return () => { playerRef.current?.videojs.cloudinary.dispose(); - playerInstances = playerInstances.filter((id) => id !== playerId) + playerInstances = playerInstances.filter((id) => id !== playerId); } }, []); /** - *getPlayerRefs + * getPlayerRefs */ - function getPlayerRefs() { - return { - player: playerRef.current, - video: videoRef.current - } + return { player: playerRef.current, video: videoRef.current } } return ( @@ -127,7 +124,7 @@ const CldVideoPlayer = (props: CldVideoPlayerProps) => { -
+
); -}; +}); export default CldVideoPlayer;