-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Backward Compatibility Audit #2402
Comments
I vote to re-export |
I'm curious why |
I don't see why not export it. It could be useful. |
My concern was sort of two-fold. First, it increases the surface area of the API, which increases the cost of maintenance/support. Second, that it wasn't part of the core competency of VideoJS. It's not available via |
I was hoping EventEmitter would get exported because we've re-implemented it in videojs-contrib-media-sources and videojs-contrib-hls. That said, I agree it's outside the core competency of video.js and may not have broad applicability for plugins. |
Well, if it's being used in plugins, then I would revise my position and say that |
Our event emitter implementation is the bare minimum and it is required for the text track work. There won't be any changes to it. |
Can we at least rename EventEmitter to MockEventTarget or something else that has less expectations behind it? EventEmitter sounds like we're building all our components off it but we just use it to emulate an element for tracks. |
@misteroneill this is awesome, thanks for putting it together.
|
No problem! I've updated the tables with your feedback @heff. You're right that What do you mean by shimming |
I'm fine with renaming the EventEmitter but can we avoid using the adjective "Mock"? That's strongly associated with testing in my mind and the EventEmitter is intended for more than just unit testing. |
I came here to say I could be convinced to rename |
@misteroneill btw, vjs 4.0 had a bunch of helper functions that were available in the dev version (but not the minified version due to GCC's minification). For example |
@misteroneill in 5.0 we're no longer using Component's |
(man, I'm totally gonna change my username to misterheff. It makes every response sound so respectful!) 👍 EventTarget There's levels of backwards compatibility we can support for component initialization. |
I demand formality in all things! (Actually, it's just that I have a common name/first initial, so a bunch of usernames and domains have been long-registered) @gkatsev Interesting re: |
Ok, so, in going through 4.x's |
Most of the stuff wasn't exported, so, documentation isn't necessary. However, a lot of those are super useful and we definitely should export them. |
Alright, in that case, let's make a separate issue for things that were previously not exported and would be nice to export in 5.0. I don't want to lose focus of the purpose of this ticket - finding methods of |
For if (typeof subClassMethods.init === 'function') {
log.warn('The "init" method is deprecated in favor of "constructor".');
subClassMethods.constructor = subClassMethods.init;
delete subClassMethods.init;
} Opinions? |
Time to put my 🚲 in the shed. |
EventTarget is the name the DOM uses for it's EventEmitter implementation that DOM Elements extend from. |
@gkatsev I concede. |
I still would prefer to keep EventEmitter. |
Updated wiki document to reflect these changes/removals. |
Upon further investigation, |
Closing this in favor of PR #2406. Changes are moved to the wiki, which is the canonical source for changes/removals instead of this ticket. |
I'm doing a run-through of the 4.x documentation and objects to look for properties and methods that are missing from the 5.x work or whose fate is otherwise unclear or undocumented in the wiki. A lot of this is up for discussion, but I wanted to get it in one place where we can all see it.
Globals
vjs
videojs
PropertiesJSON
TOUCH_ENABLED
cache
media
options
players
util
videojs
Methodsbind
createTimeRange
parseUrl
round
trim
util.mergeOptions
Classes
CoreObject
EventEmitter
EventTarget
MediaTechController
Components
SeekHandle
SliderHandle
VolumeHandle
Other
Component#init
Player
cancelFullScreen
getTagSettings
isFullScreen
onDurationChange
handleTechDurationChange
onEnded
handleTechEnded
onError
handleTechError
onFirstPlay
handleTechFirstPlay
onFullscreenChange
handleFullscreenChange
/handleTechFullscreenChange
onLoadStart
handleTechLoadStart
onLoadedAllData
handleLoadedAllData
onLoadedMetaData
handleTechLoadedMetaData
onLoadedData
handleTechLoadedData
onPause
handleTechPause
onPlay
handleTechPlay
onProgress
handleTechProgress
onSeeked
handleTechSeeked
onSeeking
handleTechSeeking
onTimeUpdate
handleTechTimeUpdate
onVolumeChange
handleTechVolumeChange
onWaitEnd
onWaiting
handleTechWaiting
requestFullScreen
The text was updated successfully, but these errors were encountered: