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

Component.extend() does not call subclass constructor #2383

Closed
dmlap opened this issue Jul 20, 2015 · 7 comments
Closed

Component.extend() does not call subclass constructor #2383

dmlap opened this issue Jul 20, 2015 · 7 comments
Milestone

Comments

@dmlap
Copy link
Member

dmlap commented Jul 20, 2015

  1. Create a subclass of Tech using Tech.extend() with some fancy constructor logic
  2. Create a new instance of your subclass

Expected: your fancy constructor logic is run
Actual: Component's constructor is the only constructor that ran.

@dmlap dmlap added this to the v5.0.0 milestone Jul 20, 2015
@heff
Copy link
Member

heff commented Jul 20, 2015

Extending a component changed. Use the videojs.extends() function now.

We should add a warning to Component.extend. It could be possible to shim the old way in 5.0 if it's worth it, but wouldn't be very pretty.

@dmlap
Copy link
Member Author

dmlap commented Jul 20, 2015

The base tech tests are using the deprecated way, then.

@heff
Copy link
Member

heff commented Jul 20, 2015

Yeah, that needs to be updated. And I believe that should be failing in IE8.

On Mon, Jul 20, 2015 at 12:43 PM, David LaPalomento <
notifications@github.com> wrote:

The base tech tests are using the deprecated way
https://github.com/videojs/video.js/blob/master/test/unit/tech/tech.test.js#L105,
then.


Reply to this email directly or view it on GitHub
#2383 (comment).

@misteroneill
Copy link
Member

@dmlap To be clear, how were you adding "fancy constructor logic" via Component.extend - do you mean passing an init method to extend (in the old style)?

@heff
Copy link
Member

heff commented Jul 21, 2015

@misteroneill that would be my guess what he means. In the old way it's the only way to do that.

@misteroneill
Copy link
Member

After discussing in person, it seems his concern was related to doing Tech.extend(); instead of videojs.extends(Tech).

Making Component.extend work like videojs.extends would be a change to a deprecated function; so, we thought the best way forward was to make sure uses of Component.extend were removed and a deprecation warning was added.

@heff
Copy link
Member

heff commented Jul 21, 2015

Sounds good. And just to be clear, Tech.extend (and extend for any other component besides Component) aren't just deprecated, they no longer work.

@mmcc mmcc closed this as completed Aug 7, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants