Require videojs-vtt.js via require rather than concat
#3919
Conversation
use aliasify to include/exclude vtt.js require allow fallback to old script vtt include fix a bug where shell:babel was passing --w rather than -w or --watch to babel
|
As you mentioned @brandonocasey, we should look at how safe it is to bring this in as a minor since it's theoretically possible. Would make it a bit easier for current suers. |
| @@ -326,32 +318,39 @@ module.exports = function(grunt) { | |||
| } | |||
| }, | |||
| browserify: { | |||
| options: browserifyGruntOptions(), | |||
| build: { | |||
There was a problem hiding this comment.
so, build is now the main task and takes the job of both build and dist?
There was a problem hiding this comment.
Yeah, the difference between the two was that dist would replace the vtt.js string path with a url to the cdn. Since we will require vtt.js by default when we build it isn't really needed to do them separately
| buildDependents.map(task => task === 'browserify:build' ? 'browserify:dist' : task) | ||
| ); | ||
|
|
||
| grunt.registerTask('build:dist', buildDependents.map(task => task)); |
There was a problem hiding this comment.
We don't need to map here anymore. Or we could just get rid of it this task.
There was a problem hiding this comment.
we may just want to get rid of the task
| */ | ||
| addWebVttScript_() { | ||
| if (!window.WebVTT && this.el().parentNode !== null && this.el().parentNode !== undefined) { | ||
| const vtt = require('videojs-vtt.js'); |
There was a problem hiding this comment.
so, basically, this requires vttjs and then below, we make sure that it becomes available globally unless the vttjs option was provided. In the novtt build, a transform tells browserify not to include this file in the output?
There was a problem hiding this comment.
Yeah, we use the vtt.js option if we have it, if not we use the require if it is available (which it won't be for novtt builds), otherwise we use the cdn url.
The transform that you mention works like this:
aliasify when passed false for a package alias will turn the require call into an empty object {} before browserify includes in in the bundle.
There was a problem hiding this comment.
Cool, just wanted to confirm I'm understanding it correctly.
| */ | ||
| addWebVttScript_() { | ||
| if (!window.WebVTT && this.el().parentNode !== null && this.el().parentNode !== undefined) { | ||
| const vtt = require('videojs-vtt.js'); | ||
|
|
||
| // load via require if avialable and vtt.js script location |
There was a problem hiding this comment.
av*i*alable to ava*i*lable (i after the a).
|
@gkatsev fixed the typo and removed |
Description
Use require on
videojs-vtt.jsrather than adding on to the end of the bundle after building.This PR replaces #3794
Specific Changes
build/grunt.jsfor browserifyaliasifyto excludevideojs-vtt.jsfor novtt buildsshell:babelwas passing--wto babel rather than-wor--watchRequirements Checklist