fix!(Video): upgrade react-player to v3 and split Video out of the main barrel

.nx/version-plans/version-plan-1791216778502.md

@@ -0,0 +1,8 @@
+---
+gamut: major
+---
+
+Move Video out of barrel and update
+
+- `Video` is now imported from `@codecademy/gamut/Video` instead of `@codecademy/gamut`.
+- `@vidstack/react` and `react-player` are now peer dependencies. If you use `Video` (or the Markdown `Iframe`/`MarkdownVideo` overrides), install them: `yarn add @vidstack/react@~1.12.12 react-player@^3.4.0`. Gamut supports `@vidstack/react` 1.12.x only; note that its npm `latest` tag still points to 0.6.x.

package.json

@@ -6,7 +6,7 @@
   "dependencies": {
     "@emotion/react": "11.14.0",
     "@emotion/styled": "11.14.1",
-    "@vidstack/react": "^1.12.12",
+    "@vidstack/react": "~1.12.12",
     "core-js": "3.7.0",
     "lodash": "^4.17.23",
     "react": "^19.0.0",

packages/code-connect/Molecules/Video.figma.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { Video } from '@codecademy/gamut';
+import { Video } from '@codecademy/gamut/Video';
 import figma from '@figma/code-connect';
 
 /**

packages/gamut/Video/package.json

@@ -0,0 +1,5 @@
+{
+  "main": "../dist/Video/index.js",
+  "module": "../dist/Video/index.js",
+  "types": "../dist/Video/index.d.ts"
+}

packages/gamut/__tests__/__snapshots__/gamut.test.ts.snap

@@ -132,6 +132,5 @@ exports[`Gamut Exported Keys 1`] = `
   "useIsMounted",
   "useLocalQuery",
   "useSubmitState",
-  "Video",
 ]
 `;

packages/gamut/jest.config.ts

@@ -9,5 +9,7 @@ export default base('gamut', {
   setupFiles: ['<rootDir>/../../script/jest/base-setup.js'],
   setupFilesAfterEnv: ['<rootDir>/../../script/jest/rtl-setup.js'],
   moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
-  transformIgnorePatterns: ['node_modules/(?!(@vidstack/react|@formatjs)/)'],
+  transformIgnorePatterns: [
+    'node_modules/(?!(@vidstack/react|@formatjs|react-player)/)',
+  ],
 });

packages/gamut/package.json

@@ -13,7 +13,6 @@
     "@formatjs/intl-locale": "5.3.1",
     "@react-aria/interactions": "3.25.0",
     "@types/marked": "^4.0.8",
-    "@vidstack/react": "^1.12.12",
     "classnames": "^2.2.5",
     "framer-motion": "^12.0.0",
     "html-to-react": "^1.6.0",
@@ -24,16 +23,20 @@
     "react-aria-components": "1.7.1",
     "react-focus-on": "3.10.0",
     "react-hook-form": "^7.71.2",
-    "react-player": "^2.16.0",
     "react-select": "^5.2.2",
     "react-truncate-markup": "^5.1.2",
     "react-use": "^17.6.1",
     "sanitize-markdown": "^2.6.7"
   },
+  "devDependencies": {
+    "@vidstack/react": "~1.12.12",
+    "react-player": "^3.4.0"
+  },
   "files": [
     "dist",
     "bin",
-    "agent-tools"
+    "agent-tools",
+    "Video"
   ],
   "license": "MIT",
   "main": "./dist/index.js",
@@ -44,9 +47,11 @@
   "peerDependencies": {
     "@emotion/react": "^11.4.0",
     "@emotion/styled": "^11.3.0",
+    "@vidstack/react": "~1.12.12",
     "css-loader": ">=4.0.0",
     "react": "^17.0.2 || ^18.3.0 || ^19.0.0",
-    "react-dom": "^17.0.2 || ^18.3.0 || ^19.0.0"
+    "react-dom": "^17.0.2 || ^18.3.0 || ^19.0.0",
+    "react-player": "^3.4.0"
   },
   "peerDependenciesMeta": {
     "css-loader": {

packages/gamut/src/Markdown/__tests__/Markdown.test.tsx

@@ -5,18 +5,23 @@ import { screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import * as React from 'react';
 
+import { Iframe, MarkdownVideo } from '../../Video';
 import { Markdown } from '../index';
 
 const mockTitle = 'a fake youtube';
 
 jest.mock('react-player', () => ({
   __esModule: true,
-  default: () => <iframe title={mockTitle} />,
+  default: ({ title }: { title?: string }) => (
+    <iframe title={title ?? mockTitle} />
+  ),
 }));
 
 jest.mock('@vidstack/react', () => ({
   __esModule: true,
-  MediaPlayer: () => <iframe title={mockTitle} />,
+  MediaPlayer: ({ title }: { title?: string }) => (
+    <iframe title={title ?? mockTitle} />
+  ),
   MediaProvider: () => <div />,
   Poster: jest.fn(),
   Track: jest.fn(),
@@ -58,7 +63,7 @@ const vimeoMarkdown = `
 `;
 
 const videoMarkdown = `
-<video src="/example.webm" title="video" />
+<video src="/example.webm" />
 `;
 
 const videoSourceMarkdown = `
@@ -119,30 +124,95 @@ describe('<Markdown />', () => {
     ).toEqual(0);
   });
 
-  it('Renders YouTube iframes using the Video component', () => {
-    renderView({ text: youtubeMarkdown });
+  it('Renders YouTube iframes using the Video component when opted in', () => {
+    renderView({
+      iframeOverride: { component: Iframe },
+      text: youtubeMarkdown,
+    });
     screen.getByTitle(mockTitle);
   });
 
-  it('Renders Vimeo iframes using the Video component', () => {
-    renderView({ text: vimeoMarkdown });
+  it('Renders Vimeo iframes using the Video component when opted in', () => {
+    renderView({
+      iframeOverride: { component: Iframe },
+      text: vimeoMarkdown,
+    });
     screen.getByTitle(mockTitle);
   });
 
-  it('Renders video tags using the Video component if they have an src', () => {
-    renderView({ text: videoMarkdown });
-    screen.getByTitle(mockTitle);
+  it('Renders bare iframes when not opted in', () => {
+    renderView({ text: youtubeMarkdown });
+    expect(document.querySelectorAll('iframe').length).toEqual(1);
+    expect(screen.queryByTitle(mockTitle)).not.toBeInTheDocument();
   });
 
-  it('Renders video tags using the Video component if they have a source', () => {
-    renderView({ text: videoSourceMarkdown });
+  it('Keeps the title attribute on bare iframes and video tags', () => {
+    renderView({
+      text: `<iframe src="https://player.vimeo.com/video/1218916076" title="Doug Workshop Intro"></iframe>\n\n<video src="/example.webm" title="Example clip"></video>`,
+    });
+    screen.getByTitle('Doug Workshop Intro');
+    screen.getByTitle('Example clip');
+  });
+
+  it('Passes the iframe title through to the Video component when opted in', () => {
+    renderView({
+      iframeOverride: { component: Iframe },
+      text: `<iframe src="https://player.vimeo.com/video/1218916076" title="Doug Workshop Intro"></iframe>`,
+    });
+    screen.getByTitle('Doug Workshop Intro');
+    expect(screen.queryByTitle(mockTitle)).not.toBeInTheDocument();
+  });
+
+  it('Does not wrap a single-line video override in a paragraph', () => {
+    renderView({
+      text: `<video src="https://example.com/clip.mp4" controls></video>`,
+      videoOverride: { component: MarkdownVideo },
+    });
+    expect(document.querySelector('p div')).toBeNull();
+  });
+
+  it('Passes undefined width and height to Video when the video has none', () => {
+    // React only warns about NaN styles once per value, so check the props directly
+    const element = MarkdownVideo({
+      src: 'https://example.com/clip.mp4',
+    } as Parameters<typeof MarkdownVideo>[0]) as React.ReactElement<{
+      width?: number;
+      height?: number;
+    }>;
+    expect(element.props.width).toBeUndefined();
+    expect(element.props.height).toBeUndefined();
+  });
+
+  it('Passes the video title through to the Video component when opted in', () => {
+    renderView({
+      text: `<video src="/example.webm" title="Example clip"></video>`,
+      videoOverride: { component: MarkdownVideo },
+    });
+    screen.getByTitle('Example clip');
+    expect(screen.queryByTitle(mockTitle)).not.toBeInTheDocument();
+  });
+
+  it('Renders video tags using the Video component if they have an src when opted in', () => {
+    renderView({
+      text: videoMarkdown,
+      videoOverride: { component: MarkdownVideo },
+    });
     screen.getByTitle(mockTitle);
   });
-  it('Renders YouTube iframes using the Video component', () => {
-    renderView({ text: youtubeMarkdown });
+
+  it('Renders video tags using the Video component if they have a source when opted in', () => {
+    renderView({
+      text: videoSourceMarkdown,
+      videoOverride: { component: MarkdownVideo },
+    });
     screen.getByTitle(mockTitle);
   });
 
+  it('Renders bare video tags when not opted in', () => {
+    renderView({ text: videoMarkdown });
+    expect(document.querySelectorAll('video').length).toEqual(1);
+    expect(screen.queryByTitle(mockTitle)).not.toBeInTheDocument();
+  });
   it('Wraps the markdown in a div by default (block)', () => {
     renderView({ text: basicMarkdown });
     expect(

packages/gamut/src/Markdown/index.tsx

@@ -12,18 +12,17 @@ import {
   createInputOverride,
   createTagOverride,
   createVideoOverride,
+  MarkdownOverrideSetting,
   MarkdownOverrideSettings,
   standardOverrides,
 } from './libs/overrides';
 import { MarkdownCheckbox } from './libs/overrides/Checkbox';
 import { Details } from './libs/overrides/Details';
-import { Iframe } from './libs/overrides/Iframe';
 import {
   MarkdownAnchor,
   MarkdownAnchorProps,
 } from './libs/overrides/MarkdownAnchor';
 import { Table } from './libs/overrides/Table';
-import { MarkdownVideo } from './libs/overrides/Video';
 import { createPreprocessingInstructions } from './libs/preprocessing';
 import { defaultSanitizationConfig } from './libs/sanitizationConfig';
 import { markdownStyles } from './styles';
@@ -36,6 +35,20 @@ const preprocessingInstructions = createPreprocessingInstructions();
 
 const isValidNode = () => true;
 
+/**
+ * Puts each <video> block on its own lines so marked emits an HTML block.
+ * A single-line <video>...</video> is parsed as paragraph text, which wraps
+ * the video's block-level output in a <p>.
+ */
+const normalizeVideoBlocks = (text: string) =>
+  text.replace(
+    /(<video\b[^>]*>)([\s\S]*?)(<\/video>)/gi,
+    (_match, open: string, inner: string, close: string) => {
+      const body = inner.trim();
+      return `\n\n${open}\n${body ? `${body}\n` : ''}${close}\n\n`;
+    }
+  );
+
 const MarkdownWrapper = styled.div<{ spacing: 'loose' | 'tight' | 'none' }>`
   ${({ theme, spacing }) => {
     const spacingStyleFunction = markdownStyles[spacing];
@@ -47,9 +60,7 @@ export type SkipDefaultOverridesSettings = {
   a?: boolean;
   checkbox?: boolean;
   details?: boolean;
-  iframe?: boolean;
   table?: boolean;
-  video?: boolean;
 };
 
 export type MarkdownProps = {
@@ -68,6 +79,18 @@ export type MarkdownProps = {
    * Callback when a markdown anchor tag is clicked
    */
   onAnchorClick?: (event: React.MouseEvent<HTMLAnchorElement>) => void;
+  /**
+   * Renders <iframe> tags with the given component (e.g. Iframe from
+   * '@codecademy/gamut/Video') instead of a bare iframe. Opt-in so Markdown
+   * doesn't statically depend on react-player unless you need it.
+   */
+  iframeOverride?: MarkdownOverrideSetting;
+  /**
+   * Renders <video> tags with the given component (e.g. MarkdownVideo from
+   * '@codecademy/gamut/Video') instead of a bare video tag. Opt-in so
+   * Markdown doesn't statically depend on react-player unless you need it.
+   */
+  videoOverride?: MarkdownOverrideSetting;
 };
 
 export class Markdown extends PureComponent<MarkdownProps> {
@@ -81,6 +104,8 @@ export class Markdown extends PureComponent<MarkdownProps> {
       inline = false,
       headerIds = true,
       onAnchorClick,
+      iframeOverride,
+      videoOverride,
     } = this.props;
 
     if (!text) return null;
@@ -94,10 +119,7 @@ export class Markdown extends PureComponent<MarkdownProps> {
 
     const processingInstructions = [
       ...overrides,
-      !skipDefaultOverrides.iframe &&
-        createTagOverride('iframe', {
-          component: Iframe,
-        }),
+      iframeOverride && createTagOverride('iframe', iframeOverride),
       !skipDefaultOverrides.a &&
         createTagOverride('a', {
           component: MarkdownAnchor,
@@ -122,10 +144,8 @@ export class Markdown extends PureComponent<MarkdownProps> {
           component: Table,
           allowedAttributes: ['style'],
         }),
-      !skipDefaultOverrides.video &&
-        createVideoOverride('video', {
-          component: MarkdownVideo,
-        }),
+      videoOverride &&
+        createVideoOverride({ tagName: 'video', Override: videoOverride }),
       !skipDefaultOverrides.details &&
         createTagOverride('details', {
           component: Details,
@@ -148,7 +168,7 @@ export class Markdown extends PureComponent<MarkdownProps> {
     // Render markdown to html
     const rawHtml = inline
       ? marked.parseInline(text, markedOptions)
-      : marked(text, markedOptions);
+      : marked(normalizeVideoBlocks(text), markedOptions);
 
     const sanitizationConfig = {
       ...defaultSanitizationConfig,

packages/gamut/src/Markdown/libs/overrides/Video/index.tsx

@@ -19,12 +19,12 @@ export const MarkdownVideo: React.FC<MarkdownVideoProps> = (props) => {
     const videoProps = {
       autoplay: props?.autoPlay,
       controls: props?.controls,
-      height: Number(props?.height),
+      height: props?.height ? Number(props.height) : undefined,
       loop: props?.loop,
       muted: props?.muted,
       videoTitle: props?.title,
       videoUrl: props?.src,
-      width: Number(props?.width),
+      width: props?.width ? Number(props.width) : undefined,
       textTracks: props?.textTracks,
       showPlayerEmbed: props?.showPlayerEmbed,
     };

packages/gamut/src/Markdown/libs/overrides/index.tsx

@@ -138,11 +138,16 @@ export const createTagOverride = (
   },
 });
 
+type CreateVideoOverrideArgs = {
+  tagName: string;
+  Override: MarkdownOverrideSetting;
+};
+
 // generic video tag override
-export const createVideoOverride = (
-  tagName: string,
-  Override: MarkdownOverrideSetting
-) => ({
+export const createVideoOverride = ({
+  tagName,
+  Override,
+}: CreateVideoOverrideArgs) => ({
   shouldProcessNode(node: HTMLToReactNode) {
     if (!Override) return false;