Dependency injection (using VideoJS-Core and more)

[stevendesu]

Description

When using a module bundler like Webpack or Browserify to create a video player using videojs-contrib-ads, there's no way to specify which instance of VideoJS you wish to bind the plugin on. This has created two distinct problems for me:

Problem 1: Duplicating VideoJS

My project is organized like so:

- /project
    |-.git
    |-plugins
    |    |-pluginA
    |    |    |-.git
    |    |    |-...pluginFiles...
    |    |    |-package.json
    |    |-pluginB
    |         |-.git
    |         |-...pluginFiles...
    |         |-package.json
    |-src
    |    |-index.js
    |-package.json

Each plugin is a git submodule so that it can be developed and deployed independently, and our main player pulls in all of these submodules and combines them into a single JavaScript file for inclusion on any website. This way we can distribute a single 10KB pluginB.js for anyone who wishes to use that one plugin without pulling in the rest of our ecosystem, or we can distribute the 700KB player.js for anyone who wants all of our company's player functionality.

My problem comes in when one of our plugins depends on videojs-contrib-ads. Suppose that videojs-contrib-ads were added to the package.json for pluginA. Upon running npm install this would create a node_modules folder within pluginA. However because videojs-contrib-ads lists VideoJS as a dependency and not a peerDependency, it installs VideoJS to this node_modules folder! This means that my project now looks like this:

- /project
    |-.git
    |-node_modules
    |    |-...
    |    |-video.js/  <-----------------
    |    |-...
    |-plugins
    |    |-pluginA
    |    |    |-.git
    |    |    |-node_modules
    |    |    |    |-...
    |    |    |    |-video.js/ <--------
    |    |    |    |-...
    |    |    |-...pluginFiles...
    |    |    |-package.json
    |    |-pluginB
    |         |-.git
    |         |-...pluginFiles...
    |         |-package.json
    |-src
    |    |-index.js
    |-package.json

Now in videojs-contrib-ads you call require("video.js") which includes the closest version (/project/plugins/pluginA/node_modules/video.js). Meanwhile in src/index.js when I call require("video.js") this includes the closest version to it (/project/node_modules/video.js). Since these are two separate files, they are included as two separate modules and the videojs-contrib-ads plugin is not bound to the same instance of VideoJS as all of my other plugins, nor is it bound to the same version of VideoJS that gets invoked by src/index.js - meaning the plugin is effectively useless.

I did find a hack-y temporary solution for this problem, however, using a post-install hook:

/project/plugins/pluginA/package.json

  ...
  "scripts": {
    "postinstall": "rm -rf ./node_modules/video.js"
  },
  ...

By deleting the local video.js installation the require("video.js") call ends up recursively searching up the directory tree until it finds my root video.js installation. This eliminates the duplication, but it's a really ugly fix. I download the entire video.js library, plus all dependencies like @videojs/http-streaming, videojs-vtt.js, and @babel/runtime just to delete video.js and never use any of them. While it's ugly from a development perspective, the final bundle turns out just fine (one instance of VideoJS with all of my plugins, and videojs-contrib-ads, properly bound)

Problem 2: Always using the standard VideoJS

VideoJS comes with four different versions to meet different people's needs:

• VideoJS: Has all standard functionality
• VideoJS-Core: Removes @videojs/http-streaming (dropping HLS and DASH support) for those who wish to implement their own HLS solution, or who don't need HLS support
• VideoJS-NoVTT: Removes videojs-vtt.js (dropping WebVTT support) for those who wish to implement their own WebVTT support, or who don't need WebVTT support
• VideoJS-Core-NoVTT: Removes both @videojs/http-streaming and videojs-vtt.js

So long as I was using the standard VideoJS, everything worked fine. Recently, however, we've started to run into several issues with VideoJS HTTP Streaming. To correct this we wanted to switch to VideoJS-Core and implement HLS.js

That's when we ran into issues with our ads. videojs-contrib-ads explicitly requires video.js. It doesn't provide an option for requiring video.js/core. This means that our final bundle includes both the 361KB video.js library (to which videojs-contrib-ads binds itself) and the 176KB video.js/core library (to which all of our other plugins are bound). Our src/index.js then invokes video.js/core and none of our ads work properly, since videojs-contrib-ads is not properly bound.

Because the videojs-contrib-ads provides separate endpoints in dist/ for standard deployment (videojs-contrib-ads.js), CommonJS (videojs-contrib-ads.cjs.js), and ES Modules (videojs-contrib-ads.es.js) it may be possible to keep the current behavior for the <script> tag installation but provide a new API for people using it as a module. I believe the idea API would look something like:

// Get an instance of VideoJS
const videoJS = require("video.js/core");
// Bind videojs-contrib-ads to the instance we pass in
require("videojs-contrib-ads")(videoJS);

Steps to Reproduce

Include step-by-step instructions on how to reproduce the issue:

• Create a new project
• npm install --save video.js videojs-contrib-ads
• In your entrypoint: const videojs = require("video.js/core") and require("videojs-contrib-ads")
• Attempt to initialize the ads plugin: videojs("myPlayer", { plugins: { ads: {} } });

Expected Results

The video player is initialized

Actual Results

The ads plugins is not found

Reproducing Unit Test

Please include a unit test that fails in the most recent version of videojs-contrib-ads but should pass according to your expected results. This will help us understand what the root cause is and separate ad plugin bugs from issues with this project. A unit test can be provided like below:

I'm not familiar with QUnit or the proper format for unit tests, so someone may need to adjust this

import QUnit from 'qunit';
import VideoJS from "video.js/core";
import "videojs-contrib-ads";

QUnit.test('starts an ad break on init', function(assert) {
  const player = VideoJS("myPlayer", {
    plugins: {
      ads: {}
    }
  });
  assert.notEqual(player.ads, undefined);
});

Versions

videojs-contrib-ads version: 6.6.1

Video-js version: 7.4.1

Other plugins: None

Platforms

Browsers: All

OS/Devices: All