Remove Closure Compiler #1940
Remove Closure Compiler #1940
Conversation
Moved the vtt.js concatentation to before minification to support sourcemaps. Added a better license comment that survives minification.
| @@ -16,7 +17,12 @@ module.exports = function(grunt) { | |||
| combined = combined.replace(/GENERATED_CDN_VSN/g, grunt.vjsVersion.majorMinor); | |||
There was a problem hiding this comment.
when we get browserify, there's a transform that does this automatically for us.
There was a problem hiding this comment.
Could we still support dropping the patch number like we're doing here?
|
In the comments of the related issue we discussed changing the private vars to use the underscore-first naming convention. I can wrap that into this one, but I wanted to give people a chance to respond in case there's significant reliance on the current private vars that we should be aware of. |
|
So much red! |
| @@ -293,14 +324,25 @@ module.exports = function(grunt) { | |||
| } | |||
| }, | |||
| usebanner: { | |||
There was a problem hiding this comment.
is there a need for usebanner anymore? I believe uglify can do it for us.
There was a problem hiding this comment.
I saw that but wanted the banner in the non-minified version too, including version and copyright. Sounds like we could use browserify to do var replacement here too instead.
There was a problem hiding this comment.
We can run the unminified code through uglify beautifier if we wanted to. But makes sense to keep banner.
Does uglify create sourcemaps? If so, this could mess up with that if it is run after uglify.
There was a problem hiding this comment.
It's run before uglify, and uses the @license tag which uglify will preserve.
|
This keeps the current file naming scheme, right? |
|
It does keep the current file naming. That's a separate issue but I could easily wrap it into this one. That's one of those things I could see getting annoying when switching between 4.0 and 5.0 work, so we could also wait until closer to release. I could go either way. |
|
I added a wiki page for tracking the details (what changed and why) of any API or process changes in 5.0. If we keep this up as we go it will be easier to put together communication around the 5.0 release. |
|
lgtm |
|
🎉 |
Replace Closure Compiler with UglifyJS and remove closure compiler specific files. Addresses #1586.
We could also remove the API step in the automated tests now that we're not mangling object properties, unless we think that could still be valuable. @videojs/core-committers?