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

Provide an ES Modules build for modern browsers/tools #2910

Closed
alangdm opened this issue Jul 21, 2020 · 14 comments · Fixed by #5299
Closed

Provide an ES Modules build for modern browsers/tools #2910

alangdm opened this issue Jul 21, 2020 · 14 comments · Fixed by #5299

Comments

@alangdm
Copy link

alangdm commented Jul 21, 2020

Is your feature request related to a problem? Please describe.

This is a more detailed follow up to this comment I made on the 1.0.0 issue #2861 (comment)

hls.js's dist build currently only provides UMD builds for the different entry points and these are necessary to provide IE support by default.

However, providing an extra es modules build for all entry points would have the following benefits:

  • All modern browsers could load the es modules builds directly from a CDN using <script type="module">. This includes around 91% of all internet users
  • ES Modules builds include far less polyfills and are more easily tree-shakeable so apps using them could have between 10-30% decreases in hls.js bundle code
  • Some modern build tools like rollup, vite, or snowpack either don't support non-standard modules or only support them through extra plugins

Describe the solution you'd like
Provide es modules versions of all dist bundles and update the docs to suggest using those instead of the UMD modules if possible.

As for the names, the easiest way would be to add an .esm suffix to all the new files e.g. hls.esm.js, hls.light.min.esm.js, etc.

Describe alternatives you've considered
Considering that real es modules output support in webpack won't be included until v5, these are the approaches I was able to come up with to tackle this issue:

  1. Migrate to Webpack 5: this would arguably be the easiest path, however, Webpack 5 is still in beta so it would probably be wise to wait until it's properly released if this approach is taken.
    Pros: easy path of migration
    Cons: es modules support release would require waiting until webpack 5 officially released
  2. Migrate completely to a different build tool that supports es modules output: Replace webpack completely with rollup or other build tool that supports es modules output
    Pros: Keeps a single build tool for the whole library
    Cons: Requires reconfiguring all older build outputs plus adding all the new ones and testing that everything keeps working normally
  3. Migrate partially to a different build tool that supports es modules output: Keep using webpack for the UMD build and just use a second tool for the es modules build
    Pros: Only adds the new build configurations
    Cons: Some parts like the use of 'webworkify-webpack' on demuxer might not work on the new tool and viceversa so this solution might not be feasible at all

Additional context
I created a small proof of concept of how a rollup es modules build would work using basically the same babel config as the current webpack config.
alangdm/hls.js@master...esm-build
I can create a draft PR if that makes it easier to check

Before committing on trying to create a full on PR following one of the approaches mentioned above I would like to hear what everyone else thinks regarding this issue.

Thanks a lot for your attention and for the amazing job you've been doing with this library.

@itsjamie
Copy link
Collaborator

Thank you for the detailed options @alangdm 💯

@robwalch, what are your thoughts? I think this is important, but we're probably stretched pretty thin as far as bandwidth at this time, and waiting until Webpack 5 would probably work pretty well.

@robwalch
Copy link
Collaborator

robwalch commented Jul 22, 2020

I'm a fan of rollup, especially since we're not doing code splitting or bundling of assets like css which is the main attraction in webpack. Even doing the UMD in rollup would drop some webpack boilerplate. But v5 could change that.

In terms of timing and priority - for me - this would be a post v1.0 task. I'm open to review and merge updates against master if they improve the build and package output, well before v1 if someone wants to own this.

alangdm/hls.js@master...esm-build Looks like a good start, I just wouldn't want to maintain both bundlers.

@alangdm Did you update webpack and keep it in to compare output?

@alangdm
Copy link
Author

alangdm commented Jul 24, 2020

@robwalch

I didn't really touch the webpack config, my linter just automatically changed the indentation one time I was checking it for reference 😅

I'm willing to try creating a rollup or webpack v5 config for both the UMD and ES modules build, it seems that you think the rollup config I started working on makes sense so I could keep going with that and see if everything works properly

@tjenkinson
Copy link
Member

Rollup with both a UMD and ES output (that could be pointed at from module in the package.json) sounds great to me.

@alangdm
Copy link
Author

alangdm commented Sep 2, 2020

Just a heads up on this

I sadly haven't had time to dedicate to work on the rollup config

However, it seems that Webpack 5 is finally about to release
webpack/webpack#11406

According to that issue a RC should be out in a couple weeks at most and they're aiming for an October 10th release, so I think it might be worth to try and see if the migration path and the creation of the es modules build is easier with webpack 5 than with rollup

@aradalvand
Copy link

Any updates on this?

@TilBlechschmidt
Copy link

Is there by chance any progress on this or forks to build on? Webpack 5 has been released a year ago and it is a real hassle if not impossible with modern tooling to import Hls.js into a web project that is builder centric (at least without loading it externally by adding a script tag which is undesirable in case of e.g. SDKs which contain a video player).

If there is a clear path to solve this issue and output both a UDM and ES module, I can probably spare some time to throw together a proof of concept. However, to me it is unclear what the goal of the project actually is regarding this matter as this discussion has been dormant for over a year.

@robwalch
Copy link
Collaborator

robwalch commented Oct 4, 2022

Not a Contribution

We're still on Webpack 4 because webworkify-webpack, which dynamically creates Worker JS from Webpack modules, does not work with Webpack 5. Webpack 5's recommended Worker bundler inlines duplicates of modules used in Web Workers that would significantly increase bundle size (this was an issue in v1 release candidates if memory serves). There are ways around this like: use a fork of webworkify-webpack (may require additional changes) or setup the hls.js library to run entirely in a worker (assuming the worker can load the library from the same location as the main page). The last is problematic for users bundling the player, not to mention a non-trivial change, while webworkify-webpack is somewhat less so since the webpack boiler plate usually stays intact.

Not sure where we left off with Rollup or just adding an ES target with Webpack. We can add useful build output targets but I prefer to only maintain one bundler. If we were to make the shift to Rollup we would need to solve for the deduplication of worker modules, as well as all the other output targets and features already supported by Webpack. It's worth noting that with ES output you are on your own again when it comes to Injecting modules in the Worker.

So adding an ES target output to the Webpack config - updating Webpack if necessary and maintaining Worker support and bundle size in the UMD default output (hls.js and hls.min.js) would be a requirement for merging. Supporting Workers in the ES output would be optional.

@itsjamie
Copy link
Collaborator

itsjamie commented Oct 5, 2022

The issue with all three builders I tried (esbuild, rollup, webpack 5) was the worker module.

None of the current builders have a solution that pulls and builds the worker in a blob like webworkify-webpack. I tried to get further with the webpack 5 module output, but didn't ever get it to a state where I could open a PR.

I tried Rollup and ESBuild, and while both could output the ESM style, and webpack and rollup could both be configured to output a worker bundle inline, it was a ~400kb increase in total bundle size if you left it as a single package.

Alternatively, if you shipped two separate JS files, using a worker meant two JS files being loaded, and again, an extra ~400KB being delivered to the client.

I think the clearest actionable item would be updating the webworkify module to work with the new webpack 5 module loading code and module registry. I started (continued from another fork) this process here: https://github.com/itsjamie/webworkify-webpack, and was working through the changes to the module registry and the function signatures that it parses as runtime to construct the Blob for the worker. I got to the point that most of the code was loading into the bundle, but I was trying to figure out what parameters webpack was passing to itself in the webpack_require functions.

Where I was with testing at the time: itsjamie@6b04bfe

I had gotten to where it would build in webpack 5, and was trying to get the updated webworkify working. This is after testing both rollup and esbuild and hitting the aforementioned issues with both outputting external JS files for the workers.

@itsjamie
Copy link
Collaborator

itsjamie commented Nov 27, 2022

Rob Walch resolved the issue with webworkify by disabling one flag in minification of the build and completed the webpack 5 upgrade.

23111a3#diff-1fb26bc12ac780c7ad7325730ed09fc4c2c3d757c276c3dacc44bfe20faf166fR41

We still need a strategy for the Transmuxer if we want to offer a webpackless ESM build. Given the increased interest in WebAssembly, and a native ES6 build and the original request for a ES Modules build if anyone has ideas on how to accomplish this, please fire away!

Any solution should result with a similar bundle size.

The workerify solution makes it very difficult to achieve given the enableWorker configuration option. As we have to include the code to support enableWorker: false. Meaning that any solution that doesn't reuse the code included in the base module will automatically be much larger.

Personally, I'd be open to removing the enableWorker option and allowing folks to workize the code themselves if they wanted, but that's a pretty big change for anyone using the CDN build.

@gkatsev
Copy link
Member

gkatsev commented Feb 9, 2023

offer a webpackless ESM build

Is that really necessary? With webpack 5, it should be possible to have it output an esm build (module library type).
Theoretically, that's all that's needed, but in reality it's definitely possible more work will be needed.

Specifically, I think the main ask it to be able to do import Hls from 'hls.js' and have it work natively or without configuring a bundler to support commonjs. For that to work, it doesn't require building hls.js without webpack. It, ideally, shouldn't matter how hls.js is built, just that it provides a module file.

I tried Rollup and ESBuild, and while both could output the ESM style, and webpack and rollup could both be configured to output a worker bundle inline, it was a ~400kb increase in total bundle size if you left it as a single package.

wow, that's pretty surprising, though, I guess it's possible given the specific webpack config you have. I've generally found a reduction of ~20% filesize by going from webpack to rollup.

@itsjamie
Copy link
Collaborator

@gkatsev, re the increased size when I was doing testing trying different builds, it has to do with the fact of how HLS.js handles the web worker for the transmuxer and the enableWorker config option.

Essentially, for enableWorker: true HLS.js at runtime builds a Blob using the webpack modules wrappers and dependencies IDs and creates a worker using that Blob of code.

For enableWorker: false those same modules are used and loaded directly in the current context like normal webpack modules.

If you switch to using esm mode and the built-in Worker support, you end up with that process happening at compile time, but because we support both in and out of Worker, the code ends up duplicated. It is in a prebuilt Blob in the code that is now built at compile time, and also in the runtime webpack module cache for the non-worker mode.

I thought about simply producing two files. hls.main.js and hls.transmuxer.js and it's up to the user to either load the transmuxer in the current context, or to tell hls.js to load a URL as the worker script. That way we could drop the webpack-webworkify, and enableWorker gets switched for a worker: { protocol: 'https' | 'blob' | 'none', contents: string } that could support both blob loading, or URL loading (CORS consideration).

The only issue with this approach is that we break the nice aspect of having a single CDN resource able to either use a worker or not.

@robwalch
Copy link
Collaborator

robwalch commented Feb 22, 2023

Essentially, for enableWorker: true HLS.js at runtime builds a Blob using the webpack modules wrappers and dependencies IDs and creates a worker using that Blob of code.

Could we use this approach to build a blob from the Hls class/function a wrapper iife (provided the player module could handle worker instantiation and execution when run in a worker context)? That would allow two ways of setting up a worker: first using the blob method and second pointing a worker to the hls.*.js script. This is the approach I would prefer we take. This way we avoid adding additional script output that users could trip on setting up, and we replace webworkify with something simpler and more fool-proof (assuming stringifying the entire player module context is easy).

@robwalch
Copy link
Collaborator

Submitted #5299 to specifically address this issue and related worker issues. I've only added one esm target. Please take a look and provide feedback in the PR.

robwalch added a commit that referenced this issue Mar 17, 2023
Add an es module target (#2910)
UMD build worker injection (#5107)
Add ES5 syntax check for UMD builds (#5301, #5288)
robwalch added a commit that referenced this issue Mar 23, 2023
Add an es module target (#2910)
UMD build worker injection (#5107)
Add ES5 syntax check for UMD builds (#5301, #5288)
robwalch added a commit that referenced this issue Mar 24, 2023
Add an es module target (#2910)
UMD build worker injection (#5107)
Add ES5 syntax check for UMD builds (#5301, #5288)
robwalch added a commit that referenced this issue Mar 27, 2023
Add an es module target (#2910)
UMD build worker injection (#5107)
Add ES5 syntax check for UMD builds (#5301, #5288)
robwalch added a commit that referenced this issue Mar 27, 2023
)

* Convert build packager to rollup
* Add an es module target (#2910)
* UMD build worker injection (#5107)
* Add ES5 syntax check for UMD builds (#5301, #5288)
* Add common rollup config
* handle `self` not existing (happens in nodejs. The check that makes sure the dist file can be required in node and not throw was failing because of this.)
* Disable coverage in CI
* Add `config.workerPath` option and "hls.worker.js" build output (#5107)
* Include transmuxer-interface id ("main" and "audio") in Web Worker setup and error logs

Co-authored-by: Tom Jenkinson <tom@tjenkinson.me>
@robwalch robwalch modified the milestones: 1.5.0, 1.4.0 Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

7 participants