Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add component(1) support #1032

Merged
merged 2 commits into from
Feb 28, 2014
Merged

Add component(1) support #1032

merged 2 commits into from
Feb 28, 2014

Conversation

peteschaffner
Copy link

I have to reference video.dev.js over the minified version because the minifier seems to munge the module.exports property. This isn't a problem as minification is an app-level concern, but I wanted to bring it to your attention. I will file a separate bug for it ;)

@gkatsev
Copy link
Member

gkatsev commented Feb 24, 2014

In the newest vjs release, video.js should be the unminified version whereas video.min.js would be the minified version.
Also, I think it is better to use the unminified version. Often, if you are using something like bower or component, you already have a build system setup for minifying and concatenating, so, it would be best to just let the user handle the minifying in those cases.

@peteschaffner
Copy link
Author

Tots. Let me make the change for using video.js (don't know how I missed that).

@heff heff added confirmed and removed confirmed labels Feb 24, 2014
@peteschaffner
Copy link
Author

@gkatsev I just built on master and am still getting the old video[.dev].js files.

image dist output

Is the new file output you mentioned merged yet?

@gkatsev
Copy link
Member

gkatsev commented Feb 25, 2014

Ignore me. I guess this wasn't changed yet. Sorry for the confusion.

@peteschaffner
Copy link
Author

No problem. I think this should be good to merge then.

@heff
Copy link
Member

heff commented Feb 28, 2014

@gkatsev, sorry, #1013 is waiting for 5.0 because it's a significant change.

I do wonder if it'd be better to use the minified version since we can get better compression, and it wouldn't expose things that aren't explicitly exported. But probably not a big concern. Pulling this in.

Also the module.exports minification issue is fixed in 4.4.2 now.

We need to make sure the release process gets updated to update this file now too.

@heff heff merged commit 4b0db76 into videojs:master Feb 28, 2014
@gkatsev gkatsev mentioned this pull request Mar 31, 2014
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants