Support universal builds ("SSR")

[friday]

This PR will...

Add a guard against the global window object (not present for Node.js), in order to avoid errors when loading hls.js into Node.js with require() (including using import statements via bundlers or transpilers).

Instead of throwing an error it will export an empty object {}. In effect this is making it possible to load the module and not get any error, so the error handling can be deferred, if need be.

The guard can alternatively be added to the hls.js source code, to each file containing references to the DOM. This is less optimal imo. One of the maintainers seems to think the same (see: #1642 (comment))

Hls.js doesn't support ES6 modules that can be imported in Node.js (.mjs-files), if this is changed in the future, then this fix will not work for them. ES6 module makes this sort of fix impossible 😕 The webpack bundles uses UMD, which doesn't have this problem.

Resolves issues:

#1813 (unfixed, but closed by author)
#1642 (previous PR attempting to fix this, now reverted)

Checklist

• changes have been done against master branch, and PR does not conflict
• no commits have been done in dist folder (we will take care of updating it)
• new unit / functional tests have been added (whenever applicable) (8f09e5e)
• API or design changes are documented in API.md

[tjenkinson]

I think the problem really should be solved by the users of the library (i.e not allowing the require to happen in the first place, or making it return a stub in some way when not running in the browser), but this workaround has a super low foot print so seems fine to me 👍

Please could you git cherry-pick f8467b105578636c596a631055e1a4ec1507bc75 back though?

[tjenkinson]

please can you add the entryOnly: true option? I don't think this line needs to be included anywhere but the entry point

[friday]

@tjenkinson: Great feedback 👍 I cherry-picked your Travis test back (changed the reference to this PR) and added entryOnly: true.

I think the problem really should be solved by the users of the library (i.e not allowing the require to happen in the first place

ES6-modules must be top-level, so it's not possible to conditionally import modules. It's very common to transpile es6-modules (import to require-statements), making this a problem for require as well, even for people who doesn't even know they're using require / commonjs.

[tjenkinson]

Thanks will wait to see what others think

[tchakabam]

One simple question :) Why can't the SSR environment take care of this, instead of every DOM-based lib having to care about SSR? Somehow I can't believe that the people who made these SSR solutions didn't think about frontend libs actually relying on window and building a solution for that. Isn't there some sort of DOM-shim for your SSR env somehow?

I remember now we very recently had the exact same issue/discussion with another contributor. It would be nice if we could at least explain to ourselves exactly why a frontend lib has to take care of a non-frontend environment, while it should be that environment that takes care of the fact it is requiring frontend libs. Know what I mean? :)

[friday]

Why can't the SSR environment take care of this, instead of every DOM-based lib having to care about SSR?

I don't exactly know, since I haven't written any of them.

There's nothing inherent in SSR that causes this. The problem is that ES6 modules can't be loaded conditionally. This indirectly also affects commonjs, because of transpilation, bundling etc. Meteor and other early frameworks were able to handle SSR before ES6 modules. They had their own methods for module imports and bundling, which didn't have to adhere to these standards. Some other frameworks (like Angular) can still work around this by controlling the build. I prefer not making the assumption that they are all capable of this, or that they all want to. React's SSR implementation is just a method people can use to render to a string and serve on the server side. It's possible to create two separate entry points in order to exclude the client-only dependencies, but that could lead to quite a considerate code duplication, since you can't stuff the conditions into some isolated file and load it conditionally since conditional module loading is the problem to begin with. next.js and similar solutions that handles setting up the React SSR wrapping for you assumes you want to render the same files on the client and server, which makes sense.

It may seem possible to override the native Node.js require() method and wrap it in a try / catch, but it's not. require() is not a global method.

Eventually the dynamic-import spec could be used to avoid this. It will load modules asynchronously in the client runtime code however. Since it's not suitable for bundling, it doesn't seem to cover all cases.

I can't believe that the people who made these SSR solutions didn't think about frontend libs actually relying on window and building a solution for that

This is a mischaracterization. The fact that the problem hasn't been universally solved doesn't mean individual actors hasn't thought about it or cares. Evidence points to the contrary. If this would have been easy to "solve", it would have been solved.

I remember now we very recently had the exact same issue/discussion with another contributor.

You're referring to the other PR I linked to in the description. That was a different solution. I could quote you on that: "basically, it leaves us with having to use a wrapper around window in every possible place".

This solution doesn't have that problem. It doesn't even change the source code.

Isn't there some sort of DOM-shim for your SSR env somehow?

jsdom is intended for running client side code in Node.js, for use cases such as testing dom manipulation and rendering canvas with node-canvas and such. It's like a lightweight and much faster headless browser with no graphical capabilities (can't even take screenshots) implemented in Node.js.

This is not what you need. Unless the front-end dependency is a component or something that renders html, what you want is for it not to run on the server. It's better to opt-out completely than wasting resources on the server.

Also, adding "window" to the global namespace is a terrible idea, since it makes other modules think your environment is a browser. It would for example break the solution in this PR and other libraries that uses similar solutions, in case other browser API's (like navigator) are used unguarded.

When shimming is beneficial with JSDOM, they have their own script loading method which better emulates how browsers work and doesn't pollute the global scope with dom-shims in Node.js.

It would be nice if we could at least explain to ourselves exactly why a frontend lib has to take care of a non-frontend environment, while it should be that environment that takes care of the fact it is requiring frontend libs. Know what I mean? :)

No. I don't. This just seems dogmatic to me.

You could ask the same question about typings. Why should you add and maintain TypeScript definitions to prevent errors TypeScript users are getting? Javascript doesn't have types. If you're not using TypeScript yourself, you don't get the errors. So it's their problem right?

And if we're being purists about the Node vs browser environment, why are you distributing hls.js with the Node Package Manager, not the Browser Package Manager? ;)

If you ask me, the answer is "Because there's a demand". If you tick the right boxes you gain competitive advantages. This may or may not be one of the "right boxes", depending on a number of factors (benefit vs cost). It certainly isn't a critical one for most people, but I also don't quite see the problem.

If you don't want this, that's fine. I don't need this and have nothing to gain from this getting merged. I just wanted to help. I saw the issue and PR and knew how to. I'm a front-end library collaborator as well.

If you have concerns about compatibility, the technical implementation or aesthetics I'm open to discussion, but this kind of questions doesn't seem like a constructive use of our time imo.

I just noticed you're using Web Workers and webworkify-webpack. This is not familiar territory for me, so I can't tell if this would be a problem. It doesn't seem likely that the module definition would run inside the worker, but in case it does I could replace window with self, since self also doesn't exist in Node.js

[tchakabam]

OI can't afford time-wise to dive deeper in the topic than this, and only roughly understood why jsdom can't do the trick here for you, but there are things like domino out there which seem to intend providing that? https://github.com/fgnass/domino

Also, it seems that the Universal team is aware of the issue, and planning on building an integral solution on their side: angular/universal#992

Not wanting to be dogmatic or so, I just haven't been really convinced yet how this is something potentially many frontend libs would need to do in order to be supported by the Universal server-side rendering engine. I still think that it should be the other way round. In fact shouldn't this be a core feature of what an SSR engine would be?

We should definitely merge this if it really solves a problem for you, and since it doesn't cause any side-effects afaics :)

[friday]

but there are things like domino out there which seem to intend providing that? https://github.com/fgnass/domino

I don't see how that would solve anything. It's faster, but less feature complete.

Also, it seems that the Universal team is aware of the issue, and planning on building an integral solution on their side

This could become something useful if they truly do it and not just rely on domino. It won't be a generic "one size fits all" solution however. It wouldn't work for everyone, it would even break currently "ssr"-friendly libraries and make requests slower in some cases.

The browser API is huge, and these shims don't fully support it.

• Chrome has 1015 properties in window (including inherited properties and core language properties such as Array).
• eslint has 704 browser environment globals (+ 62 for the core language api). eslint uses sindresorhus/globals for this.
• jsdom has 344 properties in window (including inherited properties but not language properties except URL and perhaps some more).
• domino has 133 properties in window (including inherited properties but not language properties). domino seems to favor performance over compliance.

That's just the globals btw. Not their properties. I can't give a fair picture in general. Counting globals was just easy enough. domino shims navigator, but it only has a few properties like userAgent and related. It doesn't support navigator.language, navigator.mimeTypes or navigator.onLine for example.

If you shim parts of the browser API, you could break guards and you make the page load slower.

• It's possible that the guarded code checks for window, but then use localStorage for example (not supported in either dom-shim). Even if you guard for navigator, navigator.language.split('-')[0] (common way to get only the language code ex "en" or "de") will break with domino. These are well-supported in browsers, so for the browser code to work in browses there's no need to handle them conditionally. Code will break.
• Parts of the code or whole modules designed not to run in Node would slow down the SSR and need more server resources.
• For pages that can't be cached, this affects the initial page load.

hls.js takes ~0.05s to load in Node.js (just the require) in my local tests (on my laptop). With this PR it takes ~0s not to load. Shimming the dom will forfeit that performance benefit for all other front-end libraries with this type of solution. You most likely have a bunch of other dependencies too, so this could add up. For requests that can't be cached to pages with heavy dependencies, this will surely make a significant impact.

Not wanting to be dogmatic or so, I just haven't been really convinced yet how this is something potentially many frontend libs would need to do in order to be supported by the Universal server-side rendering engine. I still think that it should be the other way round. In fact shouldn't this be a core feature of what an SSR engine would be?

I've already answered this.

We should definitely merge this if it really solves a problem for you, and since it doesn't cause any side-effects afaics :)

Thanks! As mentioned I don't need it. At least not yet. I would prefer this conversation to be over soon however, since it's not a good use of my time either.

Edit: Seems it has already been marked as "ready to merge" :)

[tchakabam]

Added a note to the readme about SSR support