Improve error for missing plugin

[thijstriemstra]

When you try to load a plugin and forgot to include the actual .js file for that plugin, the following error is thrown:

TypeError: this[a] is not a function
http://cdnjs.cloudflare.com/ajax/libs/video.js/4.12.3/video.js
Line 59

Surely this can be handled and a more appropriate error message is shown.

[gkatsev]

If I'm understanding the question correctly, there isn't much we can do for better handling for this, unfortunately. JS doesn't provide a mechanism for a "method missing" so we could print a better error. However, I know that v8 and chrome are updating this error output to actually print out the name of the function. If I'm misunderstanding you, can you please provide a more detailed example of when you're getting the error?

[thijstriemstra]

This should trigger the error:

var player = videojs("myClip",
{
    controls: true,
    autoplay: true,
    loop: false,
    width: 600,
    height: 300,
    plugins: {
        myNonExistingPlugin: {
            foo: 'bar'
        }
    }
});

That triggers the error

TypeError: this[key] is not a function
http://cdnjs.cloudflare.com/ajax/libs/video.js/4.12.3/video.dev.js
Line 3793

in:

if (options['plugins']) {
      vjs.obj.each(options['plugins'], function(key, val){
        this[key](val);
      }, this);
    }

[gkatsev]

Ok, cool. So, I was partially right. We probably can do a better job when the plugins are included in the config to be initialized. We cannot do a better job when you initialize the plugin yourself like so:

var player = videojs('foo');
player.myNonExistingPlugin({
  foo: 'bar'
});

[thijstriemstra]

Sounds good. Your use-case wouldn't work for me anyway because the plugin load order gets screwed up when you assign it dynamically like that.

[gkatsev]

If you feel like opening a PR for your case, that would be awesome.

[thijstriemstra]

I've implemented a fix and now trying to add a unit test. It was giving some issues with the throws keyword so I thought I upgrade both grunt-contrib-jshint and jshint itself in my video.js checkout. Guess it only made it worse because now I'm seeing:

$ npm install jshint
npm WARN package.json karma-ie-launcher@0.1.5 No README data
npm WARN package.json karma-phantomjs-launcher@0.1.4 No README data
npm WARN package.json karma-qunit@0.1.4 No README data
npm WARN package.json karma-sauce-launcher@0.2.10 No README data
npm WARN prefer global jshint@2.6.3 should be installed with -g
jshint@2.6.3 node_modules/jshint
├── strip-json-comments@1.0.2
├── underscore@1.6.0
├── exit@0.1.2
├── console-browserify@1.1.0 (date-now@0.1.4)
├── shelljs@0.3.0
├── minimatch@1.0.0 (sigmund@1.0.0, lru-cache@2.5.0)
├── cli@0.6.5 (glob@3.2.11)
└── htmlparser2@3.8.2 (domelementtype@1.3.0, entities@1.0.0, domhandler@2.3.0, domutils@1.5.1, readable-stream@1.1.13)

$ npm install grunt-contrib-jshint
npm WARN package.json karma-ie-launcher@0.1.5 No README data
npm WARN package.json karma-phantomjs-launcher@0.1.4 No README data
npm WARN package.json karma-qunit@0.1.4 No README data
npm WARN package.json karma-sauce-launcher@0.2.10 No README data
grunt-contrib-jshint@0.11.0 node_modules/grunt-contrib-jshint
└── hooker@0.2.3

$ grunt test
Running "test" task

Running "jshint:src" (jshint) task

   src/js/core.js
     26 |var vjs = function(id, options, ready){
             ^ Redefinition of 'vjs'.
     73 |var videojs = window['videojs'] = vjs;
             ^ Redefinition of 'videojs'.
   test/unit/plugins.js
    167 |  throws(function() {
           ^ 'throws' is not defined.
   test/unit/test-helpers.js
      1 |var PlayerTest = {
             ^ Redefinition of 'PlayerTest'.
     23 |var TestHelpers = {
             ^ Redefinition of 'TestHelpers'.

>> 5 errors in 73 files
Warning: Task "jshint:src" failed. Use --force to continue.

Aborted due to warnings.

Are the current requirement versions for these dependencies too old and should they be upgraded to catch more errors like this, or is it something else?

[thijstriemstra]

I opened a PR but couldn't get the tests to run due to issue in previous comment, any hints are welcome.

[thijstriemstra]

PR is updated and ready for review.