fix(docs): split ejected React skin sample into player + component files#1588
Open
R-Delfino95 wants to merge 1 commit into
Open
fix(docs): split ejected React skin sample into player + component files#1588R-Delfino95 wants to merge 1 commit into
R-Delfino95 wants to merge 1 commit into
Conversation
The ejected React skin sample co-located the createPlayer({ features })
call and the React component in a single file. React Fast Refresh
requires a file to export only components — the Player const returned
by createPlayer broke that rule and caused HMR to bail out for users
copy-pasting the sample.
Split the output into player.ts (owns createPlayer) and
VideoPlayer.tsx / AudioPlayer.tsx (owns the component). The build
script now emits a Record<filename, source> per skin; EjectedSkin.astro
renders one tab per file with the component as the initial tab. The
e2e sync script and home-page demo were updated to read the new shape.
Closes videojs#1484
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
👷 Deploy request for vjs10-site pending review.Visit the deploys page to approve it
|
|
@R-Delfino95 is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
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.
fix(docs): split ejected React skin sample into player + component files
Closes #1484
Summary
player.ts(owns thecreatePlayer({ features })call) andVideoPlayer.tsx/AudioPlayer.tsx(owns the component). Co-locating both in one file made React Fast Refresh bail out — Fast Refresh requires a file to export only React components, and thePlayerconst returned bycreatePlayerviolated that rule.EjectedSkin.astronow renders one tab per file (component file first,player.tssecond, CSS last). Filename → Shiki language is inferred from the extension so.tsfiles highlight as TS instead of TSX.packages/react/src/presets/**— those already export only the component. This is purely a build-output + docs-rendering fix.any→unknown) and [Docs] Next.js ejected skin sample missing 'use client' directive #1486 ('use client'directive) are already closed, so this PR is scoped strictly to Docs: Ejected Skin Sample Causes Fast Refresh Warning in React #1484.Changes
Build script (
site/scripts/build-ejected-skins.ts)flattenSkinIntoPlayernow returns{ player: string; component: string }instead of a single concatenated string. Theplayercontent is a static template with the two imports and thecreatePlayercall; the component content carries'use client', the rest of the imports plusimport { Player } from './player';, and the JSX component.processReactSkinreturns{ tsx: Record<string, string>; jsx: Record<string, string> }keyed by filename (player.ts+VideoPlayer.tsxfor the TSX variant;player.js+VideoPlayer.jsxfor the JSX variant).tsxToJsxruns per-file.EjectedSkinEntry.tsx/jsxupdated toRecord<string, string>to match.Content collection schema (
site/src/content.config.ts)ejectedSkins.tsx/jsxZod schema changed fromz.string().optional()toz.record(z.string(), z.string()).optional().Docs rendering (
site/src/components/docs/EjectedSkin.astro)Object.entries(skin.tsx)to build one<Tab>+<TabsPanel>per file. Component files (*.tsx/*.jsx) sort first so they're the initial tab. The CSS tab stays at the end.langfor<ServerCode>is inferred from the filename extension via a smallEXT_TO_LANGmap (ts→'ts',tsx→'tsx', etc.).Home demo (
site/src/components/home/Demo/Demo.astro)defaultVideoReact.tsx!['VideoPlayer.tsx']!(the component file only) instead of the whole TSX string. Visually unchanged for end users — still a single code block.E2E sync (
apps/e2e/scripts/sync-ejected-skins.ts)tsxrecord shape. Writes bothplayer.tsandejected-react-video-skin.tsx(component renamed to keep the existing import path ingenerate-pages.tsstable). The component file'simport { Player } from './player'resolves to the siblingplayer.ts.Testing
pnpm -F site run ejected-skins→ 16 entries written.default-video-react.tsxhas keys['player.ts', 'VideoPlayer.tsx'].player.tsonly contains the two imports +export const Player;VideoPlayer.tsxopens with'use client', importsPlayerfrom./player, andcreatePlayerappears 0 times in the component file. Same shape for audio, minimal, and Tailwind variants.pnpm -F @videojs/e2e sync-ejected-skins→ writesplayer.ts,ejected-react-video-skin.tsx, andplayer.csstoapps/e2e/apps/vite/src/_generated/.pnpm -F site astro check→ 0 errors, 0 warnings.pnpm -F site test→ 18 files, 397 tests pass.pnpm -F site dev→/docs/framework/react/how-to/customize-skinsrenders two tabs (VideoPlayer.tsxinitial +player.ts) plus the existing CSS tab for each React skin.Note
Medium Risk
Medium risk because it changes the shape of generated
ejected-skins.jsonfor React skins and updates multiple downstream consumers (docs UI, home demo, and e2e sync) that rely on that output.Overview
React ejected-skin generation now outputs multiple files per skin: a separate
player.ts/jscontaining thecreatePlayer({ features })export and aVideoPlayer.tsx/AudioPlayer.tsxcomponent that importsPlayer, replacing the previous single-file TSX/JSX snippet.Docs and consumers are updated to match the new
Record<filename, code>shape:EjectedSkin.astrorenders one tab per React file with filename-based syntax highlighting, the home demo displays onlyVideoPlayer.tsx, the content schema validatestsx/jsxas records, and the e2e sync script now writes bothplayer.tsand the component (renamed to keep the existing e2e import path stable).Reviewed by Cursor Bugbot for commit 566cfe2. Bugbot is set up for automated code reviews on this repo. Configure here.