Hls.js: webpack packaging still broken

[blacktrash]

Building with webpack succeeds, but throws errors when used.
Additionally the errors differ depending on the node and npm versions used.

• node v5.6.0 / npm v3.6.0: 'Uncaught TypeError: Cannot read property 'isSupported' of undefined' (Hls undefined) - also with node v5.4.1 / npm v3.3.12 - if Hls.isSupported() is used in code this is fatal
• node v4.3.1 / npm v2.14.12: '[error] > error while initializing DemuxerWorker, fallback on DemuxerInline' - according to my testing so far only in debug mode and (seemingly?) non-fatal

See: https://github.com/flowplayer/flowplayer-hlsjs/issues/23

Can be verified with the Flowplayer hlsjs plugin code from this branch.

Related to:
https://github.com/dailymotion/hls.js/issues/240
and probably:
https://github.com/dailymotion/hls.js/issues/187

[mangui]

Hi @blacktrash I see that https://github.com/flowplayer/flowplayer-hlsjs/issues/23 is now closed.
is there any pending issue on webpack packaging ? or could it be work-arounded by any config option (for the webworker) ?

[blacktrash]

Since v5.6.0 and this patch https://github.com/flowplayer/flowplayer-hlsjs/commit/9ca8eb411d07d1766d22785d91d3e12120c7a6fa it works.
Not sure whether something like the additional

new webpack.NormalModuleReplacementPlugin(/^webworkify$/, 'webworkify-webpack')

is worth some documentation in other circumstances than ours, the entire npm/webpack etc. thing is deeply mysterious to me, I'm just the messenger, sorry ;-)

[R-Kurbatovvakoms]

I'm still getting error Cannot read property 'isSupported' of undefined when trying to use webpack. Tried 0.5.7 version, @blacktrash proposed solution but without success.

Seems like Hls is undefined. Node version is 4.2

[sompylasar]

0.6.2-2 gives:

WARNING in ./~/hls.js/dist/hls.js
Critical dependencies:
1:476-483 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
 @ ./~/hls.js/dist/hls.js 1:476-483

because of this:
https://github.com/dailymotion/hls.js/blob/1dec4e4c3400b8a71b8ad592f5695ac5df74992f/package.json#L15

but this seems to be unfixable :-1: because browserify is used, i.e. the package maintainer is responsible for the module bundling, not the app developer, as intended when using webpack.

[tjenkinson]

@sompylasar yes it will but does it work for you?

Refs: https://github.com/dailymotion/hls.js/issues/187 https://github.com/dailymotion/hls.js/pull/550

[sompylasar]

BTW refs https://github.com/webpack/webpack/issues/1617

[sompylasar]

@tjenkinson I've just switched from 0.5.41 to 0.6.2-2 because some internal errors were fixed there but not ported back to 0.5.x (like this one: https://github.com/dailymotion/hls.js/commit/61cedf94a10d884e6ae662ff894baf9627d39c47 ).

For 0.5.41 I used the following:

// WORKAROUND: For `hls.js` -- `webworkify` does not work with `webpack` because it uses `arguments`. @see https://github.com/dailymotion/hls.js/issues/265#issuecomment-193308293
new webpack.NormalModuleReplacementPlugin(/^webworkify$/, 'webworkify-webpack'),

[tjenkinson]

@sompylasar ok. With this change you no longer need that workaround you should be able to use hlsjs the same way as any other dependency. Does it work without that?

[sompylasar]

@tjenkinson Yes, looks like it works, but webpack spits the annoying warning about the pre-built javascript file.

[tjenkinson]

I think you might be able to disable the warning with the

module.noParse

webpack option

On 19 Jul 2016, at 15:52, John Babak [email protected] wrote:

@tjenkinson Yes, looks like it works, but webpack spits the annoying warning about the pre-built javascript file.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[sompylasar]

@tjenkinson Yes, thanks, module.noParse worked. One workaround has changed to another workaround:

        module: {
            loaders: [
                // ...
            ],
            noParse: [
                // WORKAROUND: For `hls.js >=0.6.x` -- against the `This seems to be a pre-built javascript file.` warning. @see https://github.com/dailymotion/hls.js/issues/265#issuecomment-233661596
                /\/node_modules\/hls\.js\/.+$/,
            ],
        },

[blacktrash]

@sompylasar - FYI, we still do it the previous way, but now requiring hls.js/lib/hls.js: https://github.com/flowplayer/flowplayer-hlsjs/commit/95f82ade9511e4d77c3329258fc05a6ee5f7ac04

[sompylasar]

@blacktrash thanks, I expected there should be a lib/ version, but missed it in the package.json, so I thought there is only a browserified distributable in the npm-installed package.

[blacktrash]

@sompylasar - I take everything back and claim the contrary ;-) Turns out this was not reliable, so I'm very grateful for your insistence - noParse seems to do the trick.
Thank you.

[sompylasar]

@blacktrash my insistence happened by mistake here, otherwise I would use the lib/ to bundle myself, like I do with other modules which are not browserified. Thanks for pointing out that for hls.js using lib/ version is less reliable than the dist/ version. What I feel wrong with the dist/ is that common modules, e.g. EventEmitter, do not get reused from the modules already used by my app.

[tjenkinson]

What I feel wrong with the dist/ is that common modules, e.g. EventEmitter, do not get reused from the modules already used by my app.

@sompylasar I agree. If there was a way of inlining the web worker code outside of browserify and it was a separate build step, then it should be possible to go back to using lib again, where the source files which use a web worker would have their source inlined at that stage. Then browserify would not be responsible for inlining the web workers anymore, the files browserify saw (in "lib"), would already be valid es5.

E.g the "build" package.json command would become something like

npm run babel && npm run inlineWorkers && browserify -t browserify-versionify -t [babelify] -s Hls src/index.js --debug | exorcist dist/hls.js.map -b . > dist/hls.js

If this was figured out at a later date it should be a completely transparent change, except for people who have set noParse in their webpack config because that would then need to be removed again!

[blacktrash]

I may have figured out a way by requiring hls.js/lib/index.js (?!): https://github.com/flowplayer/flowplayer-hlsjs/commit/70529238fda7c85e75ef6e071ab35cb141c3dc74 seems to work so far, doesn't complain about Hls being undefined. Basically kinda reverting https://github.com/dailymotion/hls.js/commit/c4f299959e9a2cfb0db63e221b0ee6a77fd0eb89#diff-b9cfc7f2cdf78a7f4b91a753d10865a2L15
Disclaimer: I don't know what I'm doing ;-)

[tjenkinson]

@blacktrash yes doing that essentially results in the same as before the change

[sompylasar]

EDIT: Disregard what I have written in this comment, I have forgotten to run npm install to update the packages after switching between branches. Looks like it works as expected with import Hls from 'hls.js'; and [email protected].

Something is still broken, the noParse option breaks with ReferenceError: require is not defined on these lines of my webpack-built app:

/* 354 */
/***/ function(module, exports) {

  'use strict';

  // This is mostly for support of the es6 module export
  // syntax with the babel compiler, it looks like it doesnt support
  // function exports like we are used to in node/commonjs
  module.exports = require('./hls.js').default;

/***/ },

Having an import Hls from 'hls.js'; in the source code.

[sompylasar]

Disregard what I have written in the comment above, I have forgotten to run npm install to update the packages after switching between branches. Looks like it works as expected with import Hls from 'hls.js'; and [email protected].

[tjenkinson]

There is still an issue because browserify isn't renaming the require function in the build. Webpack is being a bit smarter than browserify so there isn't an issue but for people using browserify it fails. :/

https://hlsjs.slack.com/archives/general/p1472826440000084

[tjenkinson]

I am seeing 2 options:

1. figure out how to get browserify to rename the require functions it generates to something else. This replicates webpacks functionality where it uses __webpack_require__.
2. Split the build process into 2 stages. Stage 1 would be to convert each file to es5 with babel and place in lib, and inline the webworker code somehow. Then stage 2 would be browserify that code. main would then point back at lib/index.js again which would be fine because the webworkers would be inlined so it would all be valid.

@mangui what do you think?

[shoremark]

Example of a plugin that would have this issue using browserify (linked to in the README.md) https://github.com/streamroot/videojs5-hlsjs-source-handler

[tjenkinson]

I've just been thinking more about this and trying to inline the worker code gets complicated because the worker code itself can require other files.

Essentially we'd want to:

1. Build workers to single js files with all require's removed
2. transform these files to new files which module.export a Blob with their contents. This means in the main source it would get the Blob that could go straight to new Worker(blob).
3. Transform all files to es5 and put in lib (and main would point to lib/index.js)
4. Browserify that for a dist build for web browsers.

or

if we switched to webpack we could use their official workers plugin and then all we would do is build the project with webpack (with umd support enabled) and main would then still point to dist/hls.js.

[tjenkinson]

The webpack plugin basically does the 4 steps I listed above. It will also create asset js files for backwards compatibility if the browser doesn't support Blob (i.e new Worker("/worker.js")), but it could be tweaked pretty easily to prevent this (https://github.com/webpack/worker-loader/issues/20).

[tjenkinson]

So after spending more time looking at this there is an issue with what I suggested in that all the demuxer code would be duplicated in the build. webworkify passes a copy of the function source, the code for all the modules it requires, and an the require implementation into the worker at runtime, so everything only appears once in the build it creates.

Therefore maybe the best option is figuring out how to rename require in browserify's output.
https://github.com/rse/browserify-derequire looks feasible.

[tjenkinson]

https://github.com/dailymotion/hls.js/pull/663

[AviVahl]

Guys, just checking the status of this issue.
I'm currently using the ES5 compiled code directly:

const Hls = require('hls.js/lib');
... = new Hls();

When I bundle my code with webpack I get the following:

WARNING in ./~/hls.js/lib/demux/demuxer.js
Module not found: Error: Cannot resolve module 'webworkify' in C:\projects\vidi\node_modules\hls.js\lib\demux
 @ ./~/hls.js/lib/demux/demuxer.js 46:19-40

After adding webworkify to my project (npm i webworkify -S), everything seems to work (HLS playback).
Is there any known browser/environment/usage-flow where this would fail?

Would you consider adding webworkify to hls.js as a (_regular; not dev_) dependency? This would save me having to specify it in my project (without requiring it in my own code).