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

Buffer Controller TypeScript #2073

Merged
merged 1 commit into from Jan 22, 2019
Merged

Buffer Controller TypeScript #2073

merged 1 commit into from Jan 22, 2019

Commits on Jan 21, 2019

  1. Convert buffer-controller.js to TypeScript

    Thank you to Tom Jenkinson for acting as a reviewer for this work, including writing some suggestions via the GitHub PR github.com//pull/2073. The commit history has been squashed as it was a fairly lengthly process to get everything working to support the conversion to TypeScript.
    
    Early in the history of this commit, we converted properties to be initialized to `null`. This was incorrect, as it changes the behaviour of code that uses `Object.keys` calls to check if an object has any keys on it. Before, they would be empty, but if the properties are set to null, this check is no longer valid and it changes the behaviour. This is something to look out for in the future of other conversions, as the values themselves are often times reset to the `null`, but begin as undefined.
    
    Added a common `types` folder for interfaces where the ownership of the type wasn't clear for the individual file. The default no-op logger which is the inferred usage did not have any arguments in it's method signature. By adding an argument, TypeScript is able to infer the correct interface. As well the Karma and Webpack configuration had to be updated to enable building TypeScript, and getting testing coverage information from Istanbul.
    
    Define lib "es2015" and "dom" to add HTMLMediaElement, SourceBuffer and Number.isFinite definitions to the TypeScript binary when it is doing type-checking.
    
    When attempting to lint the older version of `typescript-eslint-parser` would emit `no-undef` errors for TypeScript interfaces. Updating the package fixes this issue.
    
    Tom pointed out that we should not use global module declarations to extend default HTML types since users will eventually be importing *.d.ts files from the hls.js project and this would also extend their types. Due to this feedback switched to defining a module local type `ExtendedSourceBuffer`. Which instead extends with the `ended` prop.
    
    Event Handlers were set to be part of the private scope. While this has no effect now, it will be important to ensure other modules do not directly invoke the event handlers. Other methods that were prefixed with a `_` were also set into the private scope.
    
    Where possible, code was switched to early-exit to lower the cognitive load in conditional statements, an example of this was the doAppend method. Another cleanup statement was switching handling of error codes and the comments to be closer to the conditional to which they applied.
    
    Switched to using the Record<Key, Value> type to help support access via the index signature [key: T] syntax in the future.
    
    An issue that was fixed in this commit was that The controller previous exposed the local `pendingTracks` property through the event and did not set the buffer on `this.tracks` instead opting to set it on the individual track, which bubbled up to the pendingTracks object, whose reference was lost in the buffer-controller module. This commit changes this behaviour so that the controller publishes `tracks` from the public variable on the controller rather than the pendingTracks which are passed into createSourceBuffers. This also means that this fixes the `.buffer.buffered` being undefined on the demo page. Added unit tests to cover the new behaviour of throwing an error if createSourceBuffer is called without an attached media element.
    itsjamie committed Jan 21, 2019
    Copy the full SHA
    ea3e1f0 View commit details
    Browse the repository at this point in the history