-
Notifications
You must be signed in to change notification settings - Fork 25
feat(VIDEO-20073): video-specific config #920
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: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cld-vp-esm-pages ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for cld-video-player ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
💪
| export const videoPlayer = (id, playerOptions = {}, ready) => { | ||
| const { videoElement, options } = getConfig(id, playerOptions); | ||
| if (options.profile) { | ||
| console.warn('Profile option requires async initialization. Use cloudinary.player() instead of cloudinary.videoPlayer()'); |
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.
💪
| const cloudinaryVideoPlayerLegacyConfig = () => { | ||
| console.warn( | ||
| 'Cloudinary.new() is deprecated and will be removed. Please use cloudinary.videoPlayer() instead.' | ||
| ); | ||
| return { | ||
| videoPlayer: getVideoPlayer(config), | ||
| videoPlayers: getVideoPlayers(config) | ||
| videoPlayer, | ||
| videoPlayers | ||
| }; | ||
| }; |
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.
isnt it time to deprecate it for real?
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.
In theory, yes, absolutely.
On the other hand, this.
* chore: simplify and test build system * docs: transformations example page * chore: refactor and cleanup icons (#923)
Refactor Player Initialization API and Profile Configuration
Jira: https://cloudinary.atlassian.net/browse/VIDEO-20073
Overview
Refactors player initialization to centralize profile and video-specific configuration fetching, laying the groundwork for asset-level config support.
Prepared infrastructure for video-specific configuration fetching
This refactoring centralizes all configuration fetching logic in
src/utils/fetch-config.js, making it straightforward to support per-video configurations. The new architecture cleanly handles:The unified
fetchAndMergeConfigutility provides a single point for all config resolution, properly merging profiles with player options and maintaining analytics metadata.Key Changes
Code Organization
src/utils/fetch-config.js- centralized config fetching for profiles and asset-specific settingssrc/player.js- logic moved to appropriate locationscloudinaryConfigextraction intoextractOptions()src/index.jsAPI Clarity
videoPlayer,videoPlayers) - Simple use casesplayer,players) - Profile/config supportprofileoption usedTesting
Benefits
✅ Infrastructure ready for video-specific config expansion
✅ Cleaner code organization and maintainability
✅ Clear sync vs async API contracts
✅ Full backward compatibility
Breaking Changes
None