Expose utility functions

[heff]

I'm opening this issue to discuss the idea of exposing more utility functions, e.g. vjs.on() and vjs.obj.each(). If we decide to expose more, we'll use this issue to discuss the right way to do that.

Background

To date we've avoided exposing too many top-level utility functions because we're not trying to build and support a general use case utility library like jquery or underscore/lodash. Limiting the utility functions to just the capabilities we need in the internal video.js codebase helps keep them small, focused, and performant.

We've also never discouraged anyone from using jquery alongside video.js or even building plugins/components that require it. So "you can use jquery" has been one of the excuses for not exposing the internal utilities.

Many of these internal utility functions are already available when extending video.js components, e.g. Button.prototype.on(), but just not available to be used just on their own, e.g. vjs.on(any_element, 'event', fn).

The addition of plugins have encouraged a lot of new and interesting development on top of video.js specifically, and raises the question of whether or not it's still best to keep these functions private and require plugins to either use a lib like jquery/underscore or duplicate the same functionality manually when needed.

Discuss

• Do the old reasons for keeping utility functions private still stand up?
• Are there any other positives/negatives to either side that aren't mentioned here?

UPDATE 2015-03-16

We're now in development of v5.0 and have removed Closure Compiler's aggressive variable mangling, as well as started to add ES6 modules + Browserify. So all properties of videojs will be publicly available (not mangled) but we have new options for creating internal utility modules that don't get exposed. The conversation is still essentially the same but the implementation will look different.

[mmcc]

What other utilities are we talking about exposing and why? I can think of pretty clear use cases for exposing vjs.on() and even vjs.obj.each(), so those are pretty clear wins IMHO.

My opinion is that we should expose things that make sense as we come across them, but I can see why that kind of reactive policy potentially sucks for folks developing plugins.

[heff]

It would be everything in lib.js essentially. Though once we expose some of those I think people will naturally expect other functions that are usually included in utility libs, like a "smart" each(obj or array), which we've already had a PR for, but wouldn't be really valuable to core alone.

[gkatsev]

I think we should be exposing it all, as long as it's reasonable. Also, we should not be encouraging people to use these exposed methods, perhaps, unless they are writing plugins.
I just think that requiring vjs plugin authors to either depend on jquery/lodash or to rewrite their own things like vjs.on or other utility methods that are already in core is counterproductive.

[heff]

requiring vjs plugin authors to either depend on jquery/lodash or to rewrite their own things like vjs.on or other utility methods that are already in core is counterproductive

I agree with that.

as long as it's reasonable

Yeah, I'm wondering if we can be defining any parameters ahead of time to help it stay reasonable, and not get ourselves into a position where we're supporting yet another all-in-one utility library. Unless we're ok with that.

@dmlap, do you have any thoughts here? I know you've pushed back on taking on too much in other cases.

[dmlap]

I think opening up everything in util would be overkill. There's a decent amount of functionality in there that doesn't seem all that useful outside of video.js itself. Not obfuscating anything that has a reasonable use-case for plugin authors sounds like a good idea.

I'm definitely not in favor of becoming another all-in-one utility library :)

[heff]

Ok, I'm going to confirm this one then, and it sounds like we should still approach it on a case by case basis. Besides event listeners, if anyone has other thoughts on specific functions we should expose, list them here.

[gkatsev]

• vjs.on
• vjs.off
• vjs.one
• vjs.trigger
• vjs.createEl
• vjs.obj.create
• vjs.obj.each
• vjs.obj.merge
• vjs.obj.deepMerge
• vjs.obj.isPlain
• vjs.obj.isArray
• vjs.bind
• vjs.addClass
• vjs.removeClass
• vjs.get

[heff]

That's a good list. So the second question in this issue then is should we mimic existing API(s) with the naming. e.g. lodash. It'd be good to decide the right naming before we export and document them.

My hunch is we should mimic the lodash API for the methods that apply, especially since we've talked about pulling in a limited set of lodash functions into core.

[gkatsev]

👍 for consistency.

[dmlap]

Agreed on consistency.

[heff]

Yeah, I would say dom operations should match to jquery functions also.

Ok, then the next steps here would be:

• map these functions to lodash/jquery functions and make sure the APIs match
• export them

Would we want these at the root level (vjs.merge) or under a namespace like vjs.util.merge? I think I'd lean towards root level for simplicity.

[gkatsev]

I'd prefer to not match jquery API due to its internal and external inconsistency. Definitely lodash/native functions, though.

[dmlap]

I lean towards root for simplicity as well. If we have too many functions we're exporting there, we know we've become a utility library on accident :)

[heff]

@dmlap yeah, agreed.

@gkatsev I'm fine with improving on jQuery if there are inconsistencies in the functions we're mimicking. But otherwise part of the goal with any API is predictability, so I don't think we should ignore jQuery altogether. It'd be good to do a serious consistency check before this goes out either way, since we're mixing a bunch of different types of utility functions.

[gkatsev]

I dont think there are any functions in jquery that aren't also in lodash that we're mimicking/taking. In those cases, I'd prefer to go with lodash. If it only exists as a jQuery api, we should see if there is a relevant native function and then think about whether we want to mimic jquery or the native function.

[heff]

lodash doesn't do any event handling, dom work, or ajax. Definitely agree on prioritizing lodash and native APIs when possible.

[heff]

Wanted to make a note here that I'm leaning towards scoping the top-level event functions at minimum under the el namespace.

vjs.el.on()
vjs.el.off()

In talking through #1544, there was some confusion around whether vjs.on() supported components and elements or just elements, and if the new addition to component.on() would support just components or elements also. Scoping element operations under vjs.el I think would help distinguish between operations that happen on elements vs components.

[heff]

@videojs/core-committers we need to revisit this one now that we're in 5.0 development, and it applies to PR #1909.

Last comments in IRC were from @gkatsev and @dmlap about putting everything on the root object, videojs.something. Reason being it would force us to keep the API small and well defined.

When @mmcc and I were previously discussing we liked grouping functions under videojs.util. Partly to make it clear that videojs is not a utility library like lodash or jquery. @mmcc may be able to add other details.

With 5.0 we're making a big push improving the plugin authoring experience, so I want to make sure we're taking that into consideration with what we're doing here. For example do we add extra functions because of convenience even if they're not specifically used inside core? @dmlap and @gkatsev might actually be the best to speak to that. Would there be any utilities that would come in handy as a plugin author? Or what if you always had a reference to lodash and could expect those functions to always be there for plugins?

Finally, we get a lot of extra language features with the ES6 transition, so I'd like to have a better understanding of where our current utils overlap with ES6, and also the two with lodash. Bascially we need to come up with a final set and where the functions will be coming from.

[gkatsev]

Whatever we do expose, if it is not used internally, it shouldn't be available.

[heff]

@gkatsev your comment isn't very clear. Can you expand on that?

[gkatsev]

Updated my comment. Forgot a negation.

[carpasse]

I really like the idea of exposing the methods you mentioned.

vjs.on
vjs.off
vjs.one
vjs.trigger
vjs.createEl
vjs.obj.create
vjs.obj.each
vjs.obj.merge
vjs.obj.deepMerge
vjs.obj.isPlain
vjs.obj.isArray
vjs.bind
vjs.addClass
vjs.removeClass
vjs.get

Nowadays more and more projects are not relying on jquery or lodash (specially if they use a framework like angular or react) so to have this methods available from videojs will be mostly helpful.

[thijstriemstra]

My plugins use the vjs.formatTime method to manipulate display/remaining/duration time. Since it's not exposed in a public API I'm currently copying it into each plugin codebase which is not ideal at all. Is it possible to export this method? Also see #1922

[brianpkelley]

While I can't comment on specific methods or variables, the suite of plugins I've created rely on the end user using the dev build. At work I'm using a custom build process to package all of the plugins and minimizing (without mangling) video.js. So I'm all for exposing more.

I feel creating plugins for a video player library should be framework agnostic. Why develop plugins that require frameworks that include duplicated functionality? I'm not advocating turning video.js into a utility framework, but why restrict your plugin developers?

[dmlap]

We decided that utility methods should be directly on the videojs object.

videojs.on(...);

[heff]

Pieces of this have been merged in with #2182 and #2139. I'm going to shift the requirement to export every utility function to 5.1 so we can get 5.0 released.

[dmlap]

I agree exporting new utilities can wait for 5.1 but we've dropped quite a few of the old utilities in the transition to 5.x (#2321 and #2322, for instance). It doesn't seem like the right move to break backward compatibility for one minor release to me.

[heff]

I'm fine with moving this back to 5.0 since we're waiting for the BC player testing and migration before officially releasing 5.0.

[gkatsev]

Going to close this since with the 5.2.0 release we're exporting a bunch of utilities including some DOM ones. Moving forward, we can decide on a case by case basis whether we need to export more but I think this is largely solved.