Skip to content
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

fix: await re-creating player before updating video #363

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/react-youtube/src/YouTube.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ class YouTube extends React.Component<YouTubeProps> {
this.createPlayer();
}

componentDidUpdate(prevProps: YouTubeProps) {
async componentDidUpdate(prevProps: YouTubeProps) {
if (shouldUpdatePlayer(prevProps, this.props)) {
this.updatePlayer();
}

if (shouldResetPlayer(prevProps, this.props)) {
this.resetPlayer();
await this.resetPlayer();
}

if (shouldUpdateVideo(prevProps, this.props)) {
Expand Down
34 changes: 18 additions & 16 deletions packages/react-youtube/src/Youtube.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import '@testing-library/jest-dom';
import React from 'react';
import { render, queryByAttribute } from '@testing-library/react';
import { render, queryByAttribute, waitFor } from '@testing-library/react';
import YouTube from './YouTube';

// @ts-ignore
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('YouTube', () => {
expect(playerMock.destroy).toHaveBeenCalled();
});

it('should create and bind a new YouTube player when props.videoId, playerVars.autoplay, playerVars.start, or playerVars.end change', () => {
it('should create and bind a new YouTube player when props.videoId, playerVars.autoplay, playerVars.start, or playerVars.end change', async () => {
const { rerender } = render(
<YouTube
videoId="XxVg_s8xAms"
Expand Down Expand Up @@ -182,7 +182,7 @@ describe('YouTube', () => {
// player is destroyed & rebound, despite the changes
expect(playerMock.destroy).toHaveBeenCalled();
// and the video is updated
expect(playerMock.loadVideoById).toHaveBeenCalled();
await waitFor(() => expect(playerMock.loadVideoById).toHaveBeenCalled());
});

it('should not create and bind a new YouTube player when only playerVars.autoplay, playerVars.start, or playerVars.end change', () => {
Expand Down Expand Up @@ -260,33 +260,33 @@ describe('YouTube', () => {
expect(Player.mock.calls[0][1]).toEqual({ videoId: 'XxVg_s8xAms' });
});

it('should load a new video', () => {
it('should load a new video', async () => {
const { rerender } = render(<YouTube id="new-video" videoId="XxVg_s8xAms" />);

rerender(<YouTube id="new-video" videoId="-DX3vJiqxm4" />);

expect(Player).toHaveBeenCalled();
expect(Player.mock.calls[0][0] instanceof HTMLDivElement).toBe(true);
expect(Player.mock.calls[0][1]).toEqual({ videoId: 'XxVg_s8xAms' });
expect(playerMock.cueVideoById).toHaveBeenCalledWith({ videoId: '-DX3vJiqxm4' });
await waitFor(() => expect(playerMock.cueVideoById).toHaveBeenCalledWith({ videoId: '-DX3vJiqxm4' }));
});

it('should not load a video when props.videoId is null', () => {
it('should not load a video when props.videoId is null', async () => {
// @ts-ignore
render(<YouTube videoId={null} />);

expect(playerMock.cueVideoById).not.toHaveBeenCalled();
await waitFor(() => expect(playerMock.cueVideoById).not.toHaveBeenCalled());
});

it('should stop a video when props.videoId changes to null', () => {
it('should stop a video when props.videoId changes to null', async () => {
const { rerender } = render(<YouTube videoId="XxVg_s8xAms" />);

expect(Player).toHaveBeenCalled();

// @ts-ignore
rerender(<YouTube videoId={null} />);

expect(playerMock.stopVideo).toHaveBeenCalled();
await waitFor(() => expect(playerMock.stopVideo).toHaveBeenCalled());
});

it('should load a video with autoplay enabled', () => {
Expand All @@ -313,7 +313,7 @@ describe('YouTube', () => {
});
});

it('should load a new video with autoplay enabled', () => {
it('should load a new video with autoplay enabled', async () => {
const { rerender } = render(
<YouTube
id="should-load-new-autoplay"
Expand Down Expand Up @@ -347,10 +347,10 @@ describe('YouTube', () => {
);

expect(playerMock.cueVideoById).not.toHaveBeenCalled();
expect(playerMock.loadVideoById).toHaveBeenCalledWith({ videoId: 'something' });
await waitFor(() => expect(playerMock.loadVideoById).toHaveBeenCalledWith({ videoId: 'something' }));
});

it('should load a video with a set starting and ending time', () => {
it('should load a video with a set starting and ending time', async () => {
const { rerender } = render(
<YouTube
videoId="XxVg_s8xAms"
Expand Down Expand Up @@ -384,10 +384,12 @@ describe('YouTube', () => {
/>,
);

expect(playerMock.cueVideoById).toHaveBeenCalledWith({
videoId: 'KYzlpRvWZ6c',
startSeconds: 1,
endSeconds: 2,
await waitFor(() => {
expect(playerMock.cueVideoById).toHaveBeenCalledWith({
videoId: 'KYzlpRvWZ6c',
startSeconds: 1,
endSeconds: 2,
});
});
});

Expand Down