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

Allow video.js to be run in node #3869

Closed
nicksorrentino opened this issue Dec 16, 2016 · 8 comments · Fixed by #3889
Closed

Allow video.js to be run in node #3869

nicksorrentino opened this issue Dec 16, 2016 · 8 comments · Fixed by #3889

Comments

@nicksorrentino
Copy link

nicksorrentino commented Dec 16, 2016

Description

I am server rendering my frontend using node. We are unable to server render any modules that import video.js. Due to running document.createElement

export const BACKGROUND_SIZE_SUPPORTED = 'backgroundSize' in document.createElement('video').style;

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

  1. run node dist/video.js
  2. See stacktrace TypeError: _document2.default.createElement is not a function

Results

Expected

Video.js to not crash

Actual

It crashes running document.createElement

Error output

TypeError: _document2.default.createElement is not a function

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

what version of videojs does this occur with?
5.12.6

browsers

what browser are affected? None

OSes

what platforms (operating systems and devices) are affected? node any

cc @ljharb

@misteroneill
Copy link
Member

You should be able to require/import video.js in Node, but you shouldn't use the dist/video.js file - that is the browser-global distributable.

Rather, use es5/video.js, which is the "main" entry-point for the package (using require('video.js') should do this automatically).

@ljharb
Copy link
Contributor

ljharb commented Dec 16, 2016

That's what we're requiring - but since everything is browserified/webpacked, the same code will run in node and in the browser for us. Why is document.createElement being called without checking for its existence first, even in the browser build?

@misteroneill
Copy link
Member

@ljharb @nicksorrentino Will #3889 address your issue? It allows require('video.js') to not throw; so, modules that import video.js should be fine. However, calling videojs() will almost certainly throw outside of a browser.

@ljharb
Copy link
Contributor

ljharb commented Dec 23, 2016

@misteroneill thanks! close - i made some comments. It's totally fine for anything to throw in node as long as it's not upon require.

@misteroneill
Copy link
Member

Agreed 💯 thanks for bringing it to our attention!

gkatsev pushed a commit that referenced this issue Dec 23, 2016
Introduce the Dom.isReal() function, which makes an educated assumption about the "realness" of the document object.
Wrap code here and there with checks against Dom.isReal() as well as other defensive code.

Fixes #3869.
@gkatsev
Copy link
Member

gkatsev commented Dec 23, 2016

This should be available as part of 5.15.1, in the next tag on npm. Hopefully, it fixes these issues for good.

@ljharb
Copy link
Contributor

ljharb commented Dec 23, 2016

Thanks!

@nicksorrentino
Copy link
Author

Thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants