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

Enable building videojs with browserify. #998

Closed
wants to merge 1 commit into from

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Feb 7, 2014

This uses browserify-shim to get browserify to recognize videojs.
It uses dist/video-js/video.js for the main file of the module.
To build using browserify, you just run browserify on the module and
use --standalone (or -s) to make it produce a UMD file.
This seems to work just fine in the browser (and requirejs looks like
it'll work, though, I haven't tested it) but it won't work in node right
now because videojs is doing a lot of stuff with 'document' directly and
its causing problems in node.

This uses browserify-shim to get browserify to recognize videojs.
It uses dist/video-js/video.js for the main file of the module.
To build using browserify, you just run browserify on the module and
use --standalone (or -s) to make it produce a UMD file.
This seems to work just fine in the browser (and requirejs looks like
it'll work, though, I haven't tested it) but it won't work in node right
now because videojs is doing a lot of stuff with 'document' directly and
its causing problems in node.
@gkatsev
Copy link
Member Author

gkatsev commented Feb 7, 2014

Can be used in conjunction with grunt-browserify or something for building.
Even though we can't require it directly in node, this allows us to require vjs anyway and use it in other browserified modules that are shipped client-side.

@gkatsev gkatsev mentioned this pull request Feb 8, 2014
},
"main": "./dist/video-js/video.dev.js",
"browser": {
"videojs": "./dist/video-js/video.js"
Copy link
Member

Choose a reason for hiding this comment

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

This won't actually work right now though since we don't push anything in dist yet, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this again would depend on the exact way were publishing to npm.

@heff heff closed this in 310a063 Feb 14, 2014
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.

2 participants