Worker source handoff is fragile and likely to break with bundlers/minifiers

[issacgerges]

What version of Hls.js are you using?

1.2.9

What browser (including version) are you using?

Chrome 108.0.5359.98 (Official Build) (x86_64)

What OS (including version) are you using?

MacOS Monterey 12.6.1

Test stream

No response

Configuration

{}

Additional player setup steps

No response

Checklist

• The issue observed is not already reported by searching on Github under https://github.com/video-dev/hls.js/issues
• The issue occurs in the stable client (latest release) on https://hls-js.netlify.com/demo and not just on my page
• The issue occurs in the latest client (main branch) on https://hls-js-dev.netlify.com/demo and not just on my page
• The stream has correct Access-Control-Allow-Origin headers (CORS)
• There are no network errors such as 404s in the browser console when trying to play the stream

Steps to reproduce

1. Build hls.js with esbuild with a flag like keep-names
2. Attempt to load any stream with any config where the worker isn't forcibly disabled

Expected behaviour

The stream should load normally

What actually happened?

The stream will fail to load

Console output

Uncaught ReferenceError: __name is not defined

or, minified

Uncaught ReferenceError: t is not defined

This was previously reported in #3749, but closed as there was no repro and the report was unspecific

[issacgerges]

In order to load its worker, hls.js creates a blob url using Function.prototype.toString of a module in webpacks module cache, as well as some injected bootstrap code by a forked version of webworkify-webpack.js. The use of toString here can break in unexpected ways when a minifier modifies the code.

When using esbuild, runtime like the following is injected at the top of the script (in an iife)

  var __create = Object.create;
  var __defProp = Object.defineProperty;
  var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
  var __getOwnPropNames = Object.getOwnPropertyNames;
  var __getProtoOf = Object.getPrototypeOf;
  var __hasOwnProp = Object.prototype.hasOwnProperty;
  var __name = (target, value) => __defProp(target, "name", { value, configurable: true });

These helpers are lexically scoped to now mangled code that relies on them. Any calls to those functions added to transmuxer-worker will be broken when that modules contents are accessed as a string via toString().

When the worker loads this string via blob url, all of those functions are now undefined and will cause the runtime error described above.

This type of issue is described in the esbuild docs here but are likely not specific to only it.

I think most of this issue could be avoided by adding a workerSrc (or similar) param to the config object that allows the consumer to minify this file themselves and make it available. This file would also need to be available via a predictable entry point (like hls.js/worker)

[issacgerges]

I've provided a minimal repro here. Also, here is a report from a different library showing this class of issue can occur in Terser as well

[robwalch]

Related to

• Improve webworkify handling of named functions where name is in quotes #5036
• Fallback to inline demuxing on Worker error #5016
• Webpack 5 Upgrade #5007

[issacgerges]

Hey @robwalch, any good options here? We've got a workaround in place where we disable terser on this bundle and consume hls.min.js directly, but we'd love to move to something a bit less fragile.

[robwalch]

Hi @issacgerges,

I'd like to maintain the current ease of use at all costs.

Supporting an optional/alternative loaded worker script when defined in the config is a good idea. I would review and be open to accepting a contribution for this. Any such change should not break the default worker injection.

In terms of fragility with minifiers and manglers, I would also accept changes like #5036 that improve support for changes made to minified code with certain options enabled.

Whether the default worker injection is enabled or an optional worker script is configured, if either method failed (A: to run the worker or B: the main player could not ack it), operation should fallback to workerless mode. This was implemented in #5016. I expect HLS.js to run even with your repro steps when using the latest player version. If that is not the case, let me know. Forcing enableWorker to false should not be required.

[robwalch]

Hi @issacgerges,

I'd like to get your feedback on #5299. Would it solve or at least prevent some of the bundling issues you've described?

• Migrate to Rollup with new ESM target and UMD Worker injection #5299

cc @laurens94 and @dev-justin

[robwalch]

Version 1.4 is in beta now. We've switched from webpack to rollup for packaging. The library now includes ES module output with "hls.mjs", and a separate worker script "hls.worker.js" which you can point to via config.workerPath should you choose to use the ESM output or repackage HLS.js such that the UMD worker injection is no longer available. The worker injection is also much simpler, and the player will fallback to inline demuxing when it fails (this issue).

https://github.com/video-dev/hls.js/releases/tag/v1.4.0-beta.2