Skip to content

Conversation

@luwes
Copy link
Collaborator

@luwes luwes commented Oct 1, 2024

See plan at #1843

BREAKING CHANGES: TS, function comps, remove some providers, use media-elements, etc...

  • passing provider (custom media) settings
  • uncomment tests and make them pass
  • check docs to see missing features

@luwes luwes self-assigned this Oct 1, 2024
@luwes luwes temporarily deployed to github-preview October 1, 2024 14:30 — with GitHub Actions Inactive
@luwes luwes mentioned this pull request Oct 1, 2024
@luwes luwes temporarily deployed to github-preview March 24, 2025 19:17 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview March 24, 2025 19:17 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview April 1, 2025 18:37 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview April 1, 2025 18:37 — with GitHub Actions Inactive
@luwes luwes marked this pull request as ready for review April 1, 2025 18:39
@luwes luwes temporarily deployed to github-preview April 3, 2025 18:30 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview April 3, 2025 18:31 — with GitHub Actions Inactive
update App to use function components,
update responsive logic and docs
@luwes luwes temporarily deployed to github-preview April 4, 2025 20:49 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview April 4, 2025 20:50 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 20:02 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 20:03 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 20:52 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 20:52 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 21:13 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 21:13 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 21:46 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 2, 2025 21:46 — with GitHub Actions Inactive
@luwes luwes requested a review from Copilot May 2, 2025 21:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a major refactor for v3 of ReactPlayer with breaking changes in TypeScript, functional components, and updated build, test, and documentation processes. Key changes include the migration from legacy JS class components (removed in src/Player.js) to a new TS function component (src/Player.tsx), updates to examples and migration guides, and adjustments in build and CI/CD scripts.

Reviewed Changes

Copilot reviewed 97 out of 102 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Player.tsx New TS functional component using forwardRef with updated event logic.
src/Player.js Legacy JS class component removed.
src/HtmlPlayer.tsx Added HTML-based player component for audio/video elements.
scripts/tester/tester.js Updated testing script with minor style and environment modifications.
scripts/pre-publish.js & post-publish.js Removed pre- and post-publish scripts.
scripts/builder/builder.js Updated build configuration to support code splitting.
examples/react/src/* Updated example files to reflect v3 usage and migration of instance methods.
README.md & MIGRATING.md Documentation updated to reflect breaking changes and new configuration details.
.github/workflows/* CI/CD workflows updated to run unified build commands.
Files not reviewed (5)
  • .npmignore: Language not supported
  • biome.json: Language not supported
  • examples/react/public/App.css: Language not supported
  • examples/react/public/index.html: Language not supported
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

src/Player.tsx:15

  • The variable 'Player' shadows the component name, which can be confusing. Consider renaming it to 'ActivePlayer' to clarify its purpose.
const Player = props.activePlayer;

@luwes luwes temporarily deployed to github-preview May 8, 2025 17:33 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 8, 2025 17:34 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 8, 2025 17:36 — with GitHub Actions Inactive
@luwes luwes temporarily deployed to github-preview May 8, 2025 17:36 — with GitHub Actions Inactive
@luwes luwes merged commit a187b55 into master May 8, 2025
12 checks passed
@luwes luwes deleted the v3-refactor branch May 8, 2025 17:40
@luwes luwes mentioned this pull request May 8, 2025
@karlhorky
Copy link
Contributor

karlhorky commented Jul 2, 2025

@luwes I think this missed adding a section to MIGRATING.md noting that "Single player imports" are no longer a feature of ReactPlayer

They were originally documented in MIGRATING.md in a commit 45635ef by @cookpete :

### Single player imports

As of `v2.2`, the 🔥 __location of single player imports has changed__. Single players are not available in `v2.0` and `v2.1`.

```jsx
// Before
import ReactPlayer from 'react-player/lib/players/YouTube'

// After
import ReactPlayer from 'react-player/youtube'
```

Could this be added to the v3 migration guide section?

@karlhorky
Copy link
Contributor

karlhorky commented Jul 2, 2025

Another undocumented thing that was a surprise was that many parts of the config prop are very different (see README.md changes):

Screenshot 2025-07-02 at 14 57 04

For example, the file.forceHls config was removed - originally added in @cookpete's commit Add support for HLS and DASH streams (c28c7ff)

Would be good to have a note in the MIGRATING.md about this, and what to do if users are missing features.

@alinaserinet
Copy link

How can I set forceHLS in version 3.3.2?

@karlhorky
Copy link
Contributor

I don't know - maybe @luwes can chime in here.

But I guess the option maybe is not needed anymore? (not sure what that implies about the default behavior)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants