search

silvermine / videojs-quality-selector

MIT License
183 stars 54 forks source link

Final dist size #12

Closed EhsanCh closed 6 years ago

EhsanCh commented 6 years ago

Hello Final silvermine-videojs-quality-selector.js is big ! videojs-resolution-switcher is only 14KB (7KB minified) And this plugin is about 60KB (19kb .min) !

is there anyway to reduce its size ?

jthomerson commented 6 years ago

@EhsanCh good observation. Looking into it, this is due to our requiring underscore. Ironically, that led me to realize that we don't actually declare underscore as a dependency, which we should (it is only listed as a dev dep). I'll fix that in 1.1.1.

Yes, we could get rid of the dependency on underscore, and maybe we should. However, we (the team that maintains silvermine software) use underscore extensively on hundreds of codebases because of its ease of use. That makes me not want to get rid of it just for the sake of a minor optimization. I call this minor because: 1) video.js is nearly 700k, and 2) if people are using this to watch video, they're going to be downloading megabytes of data, so this little bit of space saving isn't really helping them.

Code and people cost more than bandwidth, so I'm inclined not to "fix" this. Feel free to re-open if 1) you have an easy API-compatible replacement (like pulling in pieces of lodash), and 2) you want to open a pull request to make that change. I'll be happy to code review and merge it if it meets that criteria and our coding standards.

jthomerson commented 6 years ago

Thanks @EhsanCh !

EhsanCh commented 6 years ago

Hi Jeremy I think it is better to not include big dependencies (underscore) in final .js file. instead should include it with Githubissues.

  • Githubissues is a development platform for aggregating issues.