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

Using videojs with webpack #2750

Closed
vimkin opened this issue Oct 27, 2015 · 41 comments
Closed

Using videojs with webpack #2750

vimkin opened this issue Oct 27, 2015 · 41 comments
Assignees

Comments

@vimkin
Copy link

vimkin commented Oct 27, 2015

Hi, I really love videojs but I would like to wonder is it even possible to use videojs with webpack?
I have installed the latest (5.0.2) version of videojs via npm.
Then I have tried to import videojs with the following line:

import videojs from 'video.js';

Which gives me an exception:
Uncaught TypeError: Cannot set property 'vttjs' of undefined
I have tried several ways to solve this, but all of them gave nothing.
Please shed some light on this issue ☀️

@chemoish
Copy link
Member

It is possible to use with webpack. However, you might need to utilize an expose-loader or imports-loader.

I am utilizing the library a bit differently, but in webpack.

import 'expose?videojs!video.js';

Maybe related: #2698

@vimkin
Copy link
Author

vimkin commented Oct 28, 2015

@chemoish I have tried expose-loader with videojs but it wasn't work. The import will succeeded in case I'm importing video.novtt.js. Think theres something wrong happens when videojs tries to import vtt.js or get WebVTT from global scope.

@doylem
Copy link

doylem commented Oct 28, 2015

I too am using webpack in my project and would like to just import videojs from "video.js"

I've tried both imports loader and expose loader, with variations with no luck, although my error message is different:

WARNING in ./~/video.js/dist/video.js
Critical dependencies:
13:480-487 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
 @ ./~/video.js/dist/video.js 13:480-487

Has anyone managed to get it going with webpack? If you have, then you have my attention 😄

@vimkin
Copy link
Author

vimkin commented Oct 28, 2015

Although there is a question how to use various plugins like: videojs.ads, videojs.vast ... with videojs while using webpack? Should I expose all of them to globals?

@gkatsev
Copy link
Member

gkatsev commented Oct 28, 2015

Sounds like webpack is complaining that vttjs is included in the videojs file but not as a dependency. I think that #2698 is a tangential issue to this one but related. It's kind of lame that webpack is producing that warning, though. But probably still more work for us to do to get this working correctly.

@chemoish
Copy link
Member

@doylem I get that same warning, but it still works. So I have ignored it for now (Least of my worries).

However, I do have multiple entry points so I do get the same warning 5 times. 😲

@chemoish
Copy link
Member

@vadikace I expose videojs and it becomes accessible to various plugins. The plugins that are also UMD must be imported appropriately.

import 'expose-loader?videojs!video.js';
import 'imports-loader?this=>window!videojs-hotkeys/videojs.hotkeys';

@doylem
Copy link

doylem commented Oct 28, 2015

Thanks for chiming in @chemoish, @gkatsev.

As an active user of videoJS (and loving it, btw) I feel that we should sort this out soonish given the amount of new projects being spun up with various react/redux boilerplate packages which almost all use webpack as a starting point.

I'm happy to help contribute, although this is not my usual area of expertise.

@gkatsev
Copy link
Member

gkatsev commented Oct 28, 2015

I don't disagree, though, I still find it weird that it works just fine in browserify but not in webpack.

@vimkin
Copy link
Author

vimkin commented Oct 29, 2015

@chemoish Yeah, also you can expose it via providePlugin

new webpack.ProvidePlugin({
      "videojs": "video.js",
      "window.videojs": "video.js"
    })

@vimkin
Copy link
Author

vimkin commented Oct 29, 2015

@gkatsev I also haven't mentioned that it works fine if I just use require('video.js'). And it gives an error only when I'm using babel-loader.

@vimkin
Copy link
Author

vimkin commented Oct 29, 2015

So this problem could be solved by including only working directory for webpack babel-loader.

@gkatsev
Copy link
Member

gkatsev commented Oct 29, 2015

Oh, interesting. What if you compile your js with babel first and then use it in webpack? Trying to eliminate variables (and trying to put off learning webpack :P).

@chemoish
Copy link
Member

@vadikace Kind of—this is the reason why I don't love webpack. There are too many ways to configure things.

Provide Plugin:

This plugin makes a module available as variable in every module. The module is required only if you use the variable.

Many plugins call videojs directly and expect it to be in the global scope. They aren't built or aware of an import system. Thus they won't have access to videojs. Also this will cause each entry to get a duplicate videojs src that will need to be consolidated (I believe).

Expose Loader:

This loader exposes the exports to a module to the global context.

@gkatsev soon you will come to the dark side~

Anticipating your findings

@gkatsev
Copy link
Member

gkatsev commented Oct 29, 2015

This thread doesn't give me much confidence in liking webpack. One of the reasons why I like browserify is that you basically don't need to configure it at all (mostly just need to tell it to use babelify).
But if someone doesn't figure out the issues, I'll definitely take a look at some point.

@amccloud
Copy link

amccloud commented Nov 4, 2015

@chemoish @gkatsev video.js is the only NPM package I've used that has had issues with webpack.

video.js actually works for me with webpack with no special configuration.

import videojs from 'video.js';

The only issue I have is this warning in the console:

./~/video.js/dist/video.js
Critical dependencies:
13:480-487 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
 @ ./~/video.js/dist/video.js 13:480-487

@amccloud
Copy link

amccloud commented Nov 4, 2015

@doylem same error message. It is actually just a warning and should be imported for you to use.

@chemoish
Copy link
Member

chemoish commented Nov 4, 2015

@amccloud same, however, I haven't interacted with any packages built with browserify (Only used it during the initial React hype).

I'd be happy if someone took a stab to figure out what makes it different. :octocat:

@thomasklein
Copy link

The comment that is left by webpack seems to be the case 'This seems to be a pre-built javascript file'. If you check out dist/video.js you will see it's full of module.exports statements and some 'resolving' references, e.g.

},{"../internal/getNative":20}],5:[function(_dereq_,module,exports){

My understanding from the 'devDependencies' entries in 'package.json' is that browserify is used to build dist/video.js. However, since there is no access to a src version of video.js from within the npm package (it contains only a 'css' folder) other module loaders, like webpack, cannot use videojs as a commonjs module. So in fact, that is the real problem: video.js is not commonjs standard the way that node.js implemented it (https://nodejs.org/api/modules.html#modules_the_module_object).

One way to solve this would normally be to just include the src/js folder into the npm package. However, since there is a dependency on babeljs it's not possible.

The only solution I see would be to remove that dependency on babeljs and rewrite the code in ECMAScript 5. Then you could easily include the src/js folder and point webpack to it. Probably not something that the video.js authors are willing to do ;)

@gkatsev
Copy link
Member

gkatsev commented Nov 23, 2015

Browserify should be creating a commonjs compatible module. The main issue I can think of is that the main video.js file includes vttjs in it.
If you require alt/video.novtt.js does webpack still complain?

@amccloud
Copy link

@thomasklein many npm packages use babel and compile the src directory to a lib or dist directory before releasing the package to npm.

@amccloud
Copy link

@gkatsev same warning when I import video.js/dist/alt/video.novtt.js

WARNING in ./~/video.js/dist/alt/video.novtt.js
Critical dependencies:
9:480-487 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
 @ ./~/video.js/dist/alt/video.novtt.js 9:480-487

@gkatsev
Copy link
Member

gkatsev commented Nov 23, 2015

In that case, it should be working unless browserify is producing an incorrect file, which I doubt. I think webpack is just producing an error because it's a pre-built file and it isn't really anything to worry about.
Or does this warning actually prevent it from working?

@amccloud
Copy link

@gkatsev browserify is likely producing the wrong file. See: https://github.com/webpack/webpack/blob/9f440e30ecac00bfc27b91d372a969d3414d194c/lib/CompatibilityPlugin.js

Edit: The link above is the source of the error.

@amccloud
Copy link

@gkatsev it works but the warning has caused confusion for people working on my project.

@gkatsev
Copy link
Member

gkatsev commented Nov 23, 2015

Eh, that's kind of lame of webpack.

Anyway, I do think we should modify our build process to use babel to compile the source to a es5/lib directory that the main file is pointing at and then still have the current dist folder that contains the current build process.

@gkatsev
Copy link
Member

gkatsev commented Nov 23, 2015

Oh maybe the issue is that browserify is producing node-compatible commonjs modules and webpack is using pure commonjs which could be causing issues.

@gkatsev
Copy link
Member

gkatsev commented Nov 23, 2015

It's not what I said in my last comment.

Webpack really shouldn't be logging that warning. So, the immediate answer is "ignore that warning".

We'll be working on revamping the build process to compile the es6 into es5 and have that be available on npm. Hopefully, we'll be able to do this without requiring a major version bump. We have no ETA for when this will happen. We have to get through some of the issues that are tagged in the 5.3 milestone probably before tackling this.

@fredguest
Copy link

@sokra thoughts on this webpack warning This seems to be a pre-built javascript file. in this particular context?

@thomasklein
Copy link

My solution for the moment is using the "script-loader" plugin on video.js.

module: 
   loaders: [
      {
                test: /video\.js/,
                loader: 'script'
      }
   ]

@mgreer
Copy link

mgreer commented Dec 15, 2015

Webpack likes to go through source files, not built files, since it rebuilds everything to allow for code-splitting, tree-shaking, etc. It warns about all such, not just videojs, so this is normal.

It would be ideal to allow for access to the source, so webpack users can leverage these advanced features (which are indeed awesome).

@hallelujah
Copy link

@mgreer as stated by @gkatsev in #3172 if you want the access to the source the best is to clone the repo separately and link it.

I understand that npm should be sufficient but anyway that works for me.

@hallelujah
Copy link

Hello,

I quickly managed integrating with webpack, it is not very difficult though you will need some tweaks.

You can check the repo of an example here: https://github.com/hallelujah/videojs-webpack-example

No need of expose or imports loader.

My 2 💲

/CC @vadikace @chemoish @doylem @gkatsev @amccloud

@anthonybarsotti
Copy link

I couldn't get this to work via any type of shimming with version 5.8.0 and webpack 1.12.14 when simply requiring it either via require('video.js') or with ES6 import syntax. I don't need vtt for my current use-case so using the webpack provide plugin to shim it via video.js/dist/alt/video.novtt works, just figured I'd share in case anyone else hits this issue and doesn't need to use vtt.

@jhnns
Copy link

jhnns commented Jul 14, 2016

It would be pretty awesome to also have the source files in the npm module 👍

Regarding this issue, I've written down an explanation how to use videojs with webpack. Please do not use the script-loader, it's not appropriate here.

@gkatsev
Copy link
Member

gkatsev commented Jul 14, 2016

Can I re-iterate that I think webpacks's warning when videojs is required is wrong? Anyway...

Thanks for the write-up @jhnns! Hopefully, it won't be needed for much longer. I'd hope to get the new build out sometime in august.

@gkatsev gkatsev self-assigned this Jul 14, 2016
@jhnns
Copy link

jhnns commented Jul 14, 2016

Can I re-iterate that I think webpacks's warning when videojs is required is wrong

Critical dependencies is certainly a bit harsh 😉. But it's true that it's a pre-built library, isn't it?

@gkatsev
Copy link
Member

gkatsev commented Jul 14, 2016

Yup, it is a pre-built file but it should be a warning at best.

@gkatsev
Copy link
Member

gkatsev commented Jul 20, 2016

I started work on this. Check it out: #3445

@gkatsev
Copy link
Member

gkatsev commented Aug 17, 2016

#3445 has been merged into master. Once a new version is released this should no longer be an issue. Closing for now since #3445 has been merged.

@IllyaMoskvin
Copy link

Disclaimer: I don't know what I'm doing. I'm new to webpack, ES6, etc.

Using @amccloud's suggestion:

import videojs from 'video.js';

...bloated my minified app.js by a whopping 3.7 MB!

Now, I don't know if that's to be expected in the webpack world, but I'm confused.

When using the CDN, video.js is only 330 KB, and video.min.js is 108 KB:

...what am I doing wrong? I'm running npm run dev, so maybe the environment is an issue, and the devDependencies are being included during build..? But even so, having to wait for 3.7+ MB to load on every page refresh is a PITA while developing. How can I stop this from happening?

@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

No branches or pull requests