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

onReady doesn't work for Facebook in ssr (different playerID) #788

Closed
shaulgo opened this issue Jan 29, 2020 · 2 comments
Closed

onReady doesn't work for Facebook in ssr (different playerID) #788

shaulgo opened this issue Jan 29, 2020 · 2 comments

Comments

@shaulgo
Copy link

shaulgo commented Jan 29, 2020

Hey, onReady doesn't work for FB player when loaded in ssr.
From debugging I found that two playerIDs are generated, one in ssr and one in client.
@cookpete What do you think about adding
componentDidMount() { this.playerId = ref.id }
If it was loaded in ssr it will change the playerId to the one the div has. Otherwise it will have no affect.
I can make pr for this.

Quick workaround is to remount reactPlayer in client (changing the key in componentDidMount)

@shaulgo
Copy link
Author

shaulgo commented Feb 4, 2020

Thanks for the fix. And amazing project (I've been using it for years).
The given solution fixes the problem. However only if you are aware of the problem and add code to use it.
It seems to me that it can be easily fixed without adding code from the outside. But maybe I'm missing something?

@cookpete
Copy link
Owner

cookpete commented Feb 4, 2020

Sorry, I should’ve followed this up after merging.

I'm not sure componentDidMount would solve the problem? Once the component is mounted it has an id set in the DOM already, and setting this.playerId won't update the DOM before the Facebook SDK load logic kicks in.

It could maybe be fixed without additional code by using react-uid within the player, but I don't really want to include react-uid in the bundle for everyone (even if you're not using Facebook URLs).

Lazy players are coming in v2.0 so I could maybe include it then.

albanqoku added a commit to albanqoku/react-player that referenced this issue Feb 24, 2021
Webmaster1116 added a commit to Webmaster1116/video-player that referenced this issue May 20, 2021
webmiraclepro added a commit to webmiraclepro/video-player that referenced this issue Sep 9, 2022
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

No branches or pull requests

2 participants