Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switched player.tech to tech_, to promote it as private #2590

Closed
wants to merge 4 commits into from

Conversation

heff
Copy link
Member

@heff heff commented Sep 15, 2015

closes #2060

Just changes player.tech to player.tech_ wherever it's used, because it's important to promote that the tech is a private reference that plugin authors shouldn't be using. This was mangled in 4.x, so should be ok to change.

@heff heff added this to the v5.0.0 milestone Sep 15, 2015
@gkatsev
Copy link
Member

gkatsev commented Sep 15, 2015

Sounds good to me.

@moay
Copy link

moay commented Sep 15, 2015

Just one question concerning this. How do I access a "non standard" tech function in a component with the tech being private?

I want to access a function of a custom tech which is not a default function like volume() or src(). I built a component which can access the player internally like this: this.player_, but if I try to access the function like this: this.player_.myFunction() it doesn't work. Neither does this.player_.tech.myFunction() (still on rc82).

How to eiter publish the function from within the tech or to access the function via some other way from the component?

Thanks. :-)

EDIT: Read the Component docs and I'm now using this.player() instead of this.player_. But this still doesn't solve the problem... I can now access the function, but with the tech being private, this isn't the right way to do it - is it?

@heff
Copy link
Member Author

heff commented Sep 15, 2015 via email

@moay
Copy link

moay commented Sep 15, 2015

Here is the basic tech (very reduced) setup:

(function() {
    var Tech = videojs.getComponent('Tech');
    var MyTech = videojs.extends(Tech, {
        myFunction: function(){
            return something;
        }
    });
    videojs.registerComponent('MyTech', MyTech);
})();

Now in my Component (simple button, ES6), I can't access this function via this.player().myFunction(). In rc82, I can do this.player().tech.myFunction() (or this.player().tech_.myFunction() for the future), but if I get the purpose of the tech being made private, this is not how it's supposed to be used. Maybe I'm just not clever enough... ;-) Is there some other way to publish a method from the tech?

@gkatsev
Copy link
Member

gkatsev commented Sep 15, 2015

In 5.0, the tech no longer has a reference to the player. But the player should still have tech_ available. And just because it's private and discourage to use, doesn't mean that you shouldn't necessarily use it. It depends on the particular usecase.
Also, for contrib-hls we ended up adding a reference to the player with the name of the tech for us to use. player.hls would reference the hls tech. Though, we're transitioning away from a tech for it and to source handlers.

@moay
Copy link

moay commented Sep 15, 2015

Okay, thanks. :)

@heff
Copy link
Member Author

heff commented Sep 15, 2015

Please don't use tech_ until you've at least submitted an issue about why you needed to use it and what the player is lacking. I haven't seen a usecase yet where we shouldn't be looking into how to expose the information at the player level, in a way that's consistent between techs (even if it's just reporting that the info/feature isn't supported).

The consistency between techs is actually growing in importance for video.js users (and users of video.js plugins), so we need to keep being religious about that.

@mmcc
Copy link
Member

mmcc commented Sep 15, 2015

lgtm

@heff heff closed this in f7466af Sep 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make player.tech a private property
4 participants