Skip to content

Commit

Permalink
remove RxJS code from the transports code
Browse files Browse the repository at this point in the history
After doing a proof-of-concept looking at how some parts of the code
looks like without RxJS (#916), this is a first functional proposal
which looks good enough to me to be merged.

It removes all RxJS code from the `transports` code in `src/transports`.

As a reminder, the reasons for doing this are:

  1. Observables are complicated and full of implicit behaviors
     (lazily running, sync or async, unsubscribing automatically after
     the last unsubscription etc.) which is difficult to reason about,
     especially for a newcomer.

     Things like exploiting schedulers through the `deferSubscriptions`
     util to work-around some subtle potential race-conditions, or
     merging Observables in a specific order for similar reasons, are
     ugly hacks that are difficult to explain to someone not familiar
     with that code.

     Even for us with multiple years of experience with it, we sometimes
     struggle with it.

  2. Promises, event listeners - and direct callbacks in general - are
     generally much more explicit and most developpers (at least JS/TS
     devs) are familiar with them.

  3. Call stacks are close to inexploitable when using RxJS.

  4. Promises provide async/await syntax which can improve drastically
     the readability of our async-heavy code, which for the moment
     suffer from callback hells almost everywhere.

However, I'm still not sure if this wish (getting rid of RxJS) is shared
by other maintainers and/or contributors, so it is still only a proposal.

Thoughts?
  • Loading branch information
peaBerberian committed Jul 27, 2021
1 parent 54ba38f commit 725f844
Show file tree
Hide file tree
Showing 50 changed files with 3,326 additions and 2,580 deletions.
6 changes: 3 additions & 3 deletions src/core/api/option_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { IRepresentationFilter } from "../../manifest";
import {
CustomManifestLoader,
CustomSegmentLoader,
ILoadedManifest,
ILoadedManifestFormat,
ITransportOptions as IParsedTransportOptions,
} from "../../transports";
import arrayIncludes from "../../utils/array_includes";
Expand Down Expand Up @@ -84,7 +84,7 @@ export interface ITransportOptions {
*/
checkMediaSegmentIntegrity? : boolean;
/** Manifest object that will be used initially. */
initialManifest? : ILoadedManifest;
initialManifest? : ILoadedManifestFormat;
/** Custom implementation for performing Manifest requests. */
manifestLoader? : CustomManifestLoader;
/** Possible custom URL pointing to a shorter form of the Manifest. */
Expand Down Expand Up @@ -272,7 +272,7 @@ interface IParsedLoadVideoOptionsBase {
url : string | undefined;
transport : string;
autoPlay : boolean;
initialManifest : ILoadedManifest | undefined;
initialManifest : ILoadedManifestFormat | undefined;
keySystems : IKeySystemOption[];
lowLatencyMode : boolean;
minimumManifestUpdateInterval : number;
Expand Down
50 changes: 0 additions & 50 deletions src/core/fetchers/manifest/get_manifest_backoff_options.ts

This file was deleted.

Loading

0 comments on commit 725f844

Please sign in to comment.