Added check for window

[mimse]

To support require in nodejs of shaka-player to fix this issue

#1445

I have added a check for window

[theodab]

I've double-checked this. It seems to work as expected; i.e. it does the same thing as the change the plyr team made.

[joeyparrish]

It seems that with this change, nothing would be imported at all. Is that intended? Why?

[mimse]

We are using shaka-player in a react environment. And we are using the same code for server-side and client-side rendering.
So when shaka-player is imported server-side window does not exist and we get an error.
We do not need video server-side so by checking for window we insure shaka-player is not imported.
Hope this make sense 😊

[joeyparrish]

I see. I think, though, that it might be a little misleading if the import fails silently. What if we import to global instead of window?

[joeyparrish]

For example, instead of checking typeof window === "object" at the top to decide to call the wrapper function, what if you changed call(window) at the bottom to call(typeof window === "object" ? window : global)? Would that work for you?

[mimse]

I will test this solution tomorrow. And get back to you. But I think it could work.

[joeyparrish]

Great, thanks!

[mimse]

@joeyparrish it worked and I have updated the PR with your feedback.

[mimse]

@joeyparrish also if the PR is accepted would it be possible for it to be cherry-picked to 2.4?

[joeyparrish]

The build bot is testing the change now. If all the tests pass, I would be happy to merge this and cherry-pick it for v2.4.1. Thank you for contributing!

[joeyparrish]

Actually, it's trivial enough that I could also cherry-pick to v2.3.x in case we do another release from that branch.

[shaka-bot]

All tests passed!

[joeyparrish]

Cherry-picked for v2.3.9 and v2.4.1.