-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
hls.js (and hls.min.js) are not being compiled to ECMAScript5 #5288
Comments
Please include the output you are seeing and samples from the builds. |
I'm not sure I understand what you are requesting. I am using the build directly from npm (I am not building hls.js myself). The issue is that the builds include syntax that is not valid ES5. I can provide the output of es-check, but if you just grep the files in dist you can find there are arrow functions ( es-check output
|
Thanks @thornbill, That's what I was looking for. I see the arrows in the webpack output and am looking into our build settings (webpack.config.js), as well as what build this regressed in. |
Regressed in v1.2.5, most likely with the Webpack 5 upgrade. |
Hi @thornbill, I didn't find an obvious solution converting Webpack 5 boilerplate to ES5. It looks like babel-loader transpiles the modules but not the boilerplate. I decided to revisit migrating to Rollup and I think it addresses the issue (see #5299). What would be great is if we had a task to run the ES5 check in the project. Would this be something you could submit a PR for? Thanks! |
Sure I would be happy to help with that. We have an automated check for Jellyfin that I should be able to adapt. |
I opened a PR that adds a build time check of ES5 syntax here: #5301 |
) * 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>
Hi @thornbill, Thank for the bug report and build time check. The fix is in beta https://github.com/video-dev/hls.js/releases/tag/v1.4.0-beta.1 |
Excellent! Thanks for the update! |
Closing as fixed in v1.4.0. |
I'm sorry to reopen this issue but I am checking and the same issue is happening over the .mjs files, they are not properly transpiled to ECMA5 and you can find out arrow functions together with const and let usage. |
@IhToN the mjs files are ES module exports. Use the UMD exports (.js files) if you want es5. |
BackstoryWe were noticing issues on newer Tizen TV's where the video would just freeze up. We don't have the exact TV's to test on, I'm assuming 2022 and up... 2017, 2018, and 2021 seemed fine during extensive testing during prime time hours. Here are the models from our customers that reported this: So we updated hls.js from The relevant issueBut then Tizen 2017 TV's stopped loading altogether with the debugger showing:
After trying all sorts of additional babel plugins and presets to no avail, I found that altering the node_modules/hls.js/package.json worked. Here is the postinstall script that we're going to use in our CI/CD: // replace 2 strings in node_modules\hls.js\package.json
// replace "mjs" with "js"
// replace "module": "./dist/hls.mjs", with "module": "./dist/hls.js", in package.json
// replace "import": "./dist/hls.mjs", with "import": "./dist/hls.js", in package.json
const fs = require('fs')
const path = require('path')
const packageJsonPath = path.join(
__dirname,
'node_modules',
'hls.js',
'package.json'
)
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8'))
packageJson.module = './dist/hls.js'
packageJson.exports['.'].import = './dist/hls.js'
fs.writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2)) This feels hacky. I'm not sure what else to do. I thought you would like to know and maybe this helps others. |
Hi @dstreet26, How are you including HLS.js in your project? This issue was fixed in 1.4.0 for the UMD builds (files ending in .js). When importing 'hls.js', the ES module is imported by default. It is then expected that your project transpiled all JavaScript to ES5. Unless, you point to one of the UMD files:
|
We were using: I didn't think to try that 🤦 I just tested |
What version of Hls.js are you using?
v1.3.4
What browser (including version) are you using?
N/A
What OS (including version) are you using?
N/A
Test stream
No response
Configuration
Additional player setup steps
No response
Checklist
Steps to reproduce
npx es-check es5 'node_modules/hls.js/dist/**/*.js'
Expected behaviour
hls.js should be compiled to ECMAScript5 as stated in the README. This is particularly important for applications supporting smart TV platforms (Tizen, WebOS) that ship old versions of browser engines that do not support ES2015+ syntax.
What actually happened?
The compiled files include arrow functions that are not supported in ECMAScript5.
Console output
Chrome media internals output
No response
The text was updated successfully, but these errors were encountered: