Enums only exported as types

[ericmatthys]

What version of Hls.js are you using?

1.4.7

What browser (including version) are you using?

Chrome 115

What OS (including version) are you using?

macOS Ventura

Test stream

N/A

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://hlsjs.video-dev.org/demo and not just on my page
• The issue occurs in the latest client (main branch) on https://hlsjs-dev.video-dev.org/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. Import the Events enum like import HlsJs, { Events } from 'hls.js';
2. Reference this enum in code like player.on(Events.MEDIA_ATTACHED, onMediaAttached);

Expected behaviour

The enum is exported as a value that can be used, not just a type.

What actually happened?

There is no type error, but at runtime, this code will error.

Console output

`Uncaught TypeError: Cannot read properties of undefined (reading 'MEDIA_ATTACHED')`

Chrome media internals output

No response

[robwalch]

Hi @ericmatthys,

Can you provide more information about your setup (TypeScript version, etc)?

As a workaround, Events is also available on the default Hls class, so in your example, you should be able to use HlsJs.Events.

[ericmatthys]

Sure, we're using TypeScript 5.1.3 currently and it's being imported into a Next.js 13.2.4 app.

HlsJs.Events works for something like player.on(HlsJs.Events.MEDIA_ATTACHED, onMediaAttached); but the Events enum is still needed for something like HlsListeners[Events.ERROR], so we'd end up using both depending on if we need it as a value or as an index type.

[robwalch]

So what you are looking for is a change in hls.ts to export Events but not strictly as a type as we do here?

hls.js/src/hls.ts

Lines 877 to 881 in c39db5b

	export type {
	MediaPlaylist,
	ErrorDetails,
	ErrorTypes,
	Events,

Would you need the same for ErrorTypes and ErrorDetails? Any other enums whose values are present in event objects that would require comparison? To be safe we could cover all enums listed in https://github.com/video-dev/hls.js/blob/v1.4.8/api-extractor/report/hls.js.api.md

[ericmatthys]

I think so. In general I'd assume enums would be exported so that they can be used as values and not purely as types.

[robwalch]

Thanks @ericmatthys,

Would you be willing to submit a PR?

[robwalch]

Hi @ericmatthys,

1. Import the Events enum like import HlsJs, { Events } from 'hls.js';
2. Reference this enum in code like player.on(Events.MEDIA_ATTACHED, onMediaAttached);

Did you ever try importing types as types separately from the library like this?

import type { Events } from 'hls.js';
import HlsJs from 'hls.js';

This works for me, since hls.js.d.ts is the default for handling types and hls.mjs the default for modules.

[ferferga]

Reading through the linked PRs, I'm seeing this was slated for 1.5.x but it's now into the 1.6.x milestone.

If possible, I'd like to have this reconsidered, since from what I'm seeing, the major hold up originally was to provide ESM, but that's already on the roadmap for 1.5.x as far I can see? (although maybe I'm missing something, sorry in that case).

Although this is minor inconvenience, the reasons to get this fixed are:

• Less boilerplate code (more concise too!)
• No warnings in one of the most popular linter stacks: eslint + eslint-plugin-import (currently this is raising warnings in the import/no-named-as-default-member rule)

[robwalch]

Hi @ferferga,

It looks like we would need to introduce a breaking change to the ESM output module (hls.mjs) that would replace the default export with a named one. Then we could include other named exports.

[robwalch]

Interested in picking up from #5685 (review) ?

[ferferga]

@robwalch Can 1.5.x live with those breaking changes?

[robwalch]

Is anyone using .mjs at runtime? If so, then no, we cannot change the default export without breaking embeds (and not just build/import statements). We'd need a way for rollup to allow a default with named exports to keep backwards compatibility. Your eslint rule will still fail unless we can have Hls in a named and default export.

So is there a way to work around this error in rollup so we can have it both ways?

[!] Error: Entry module "src/hls.ts" is using named and default exports together. Consumers of your bundle will have to use `Hls.default` to access the default export, which may not be what you want. Use `output.exports: "named"` to disable this warning

[robwalch]

So can anyone take this on?

Provide a backwards compatible build change with only minimal export changes to hls.ts. We can then set a milestone after the solution passes review.

[ferferga]

@robwalch By a quick walkthrough, it looks to me that the build process is greatly convoluted.

Is a major overhaul of the pipeline undesired? A migration to Vite maybe?

[robwalch]

Is a major overhaul of the pipeline undesired? A migration to Vite maybe?

It is undesired. Is it really necessary to resolve this issue as described? What questions do you have about the build pipeline?

Here's the default export:

hls.js/src/hls.ts

Line 39 in ef81686

export default class Hls implements HlsEventEmitter {

Type exports begin here (I don't think we need any change to these):

hls.js/src/hls.ts

Lines 931 to 934 in ef81686

	export type {
	MediaPlaylist,
	ErrorDetails,
	ErrorTypes,

build task is rollup:

hls.js/package.json

Line 34 in ef81686

"build": "rollup --config && npm run build:types",

rollup configs are defined here:

hls.js/build-config.js

Lines 309 to 312 in ef81686

	fullEsm: buildRollupConfig({
	type: BUILD_TYPE.full,
	format: FORMAT.esm,
	minified: false,

[ferferga]

@robwalch
I don't have any questions, and I think I got quite a good grasp of it by my first dive. Thank you very much for the extra insights anyway!

It's just that everything it's independent from each other, which leads to confusing behaviour, like the one this issue exposes:

• Because Typescript is just for declaration emitting, there are no warnings in the IDE about types not being exportable (with Rollup failing).

  • They are mismatched, as you can see from this issue: Typescript doesn't warn consumers about the lack of default export, because it consider it is available. But it's not.

• The variants could be simplified by having 2 entry points, each with their own proper types and import resolution when consuming.

• Vite has first-class ESM and TypeScript for typed configs and typed build scripts.

• With npm workspaces, a package for every build type can be published to be directly and explicitly pulled by consumers.

I also recall some of us (I contribute to @jellyfin) also had issues with the builds (issues with ES5 iirc) and contributed PRs, issues I also thought back at the time that were mainly due to a messy building process.

All in all, I see the current building pipeline is a castle glued with toothpaste built upon the beginning of the times, when this project was not in TS, so it's a good chance to maybe start tackling it. But, of course if its undesired, I can try to see if it can be "workarounded".

TLDR: This issue wouldn't be here in the first place if it weren't for the build process.

[robwalch]

Thanks @ferferga,

We could take changes to the build process if they were backwards compatible and solved additional needs of our users.

You did however ask if this could be included in v1.5 and the timeframe for that is short - we're almost feature complete and need to focus the beta on testing those features (not the build).

Here's a PR that adds named exports with minimal changes to the build process. Can you verify that it solves this issue for you? I managed to limit the scope to only the ESM output.

• Use named exports in ESM output #5930

Would you be willing to contribute an alternative build process / proof-of-concept that we could review? The next or following milestone (v1.6-1.7) would be appropriate for a build refactor.