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

Cannot require html5shiv in node like environments. #178

Open
DonutEspresso opened this issue Feb 27, 2015 · 6 comments
Open

Cannot require html5shiv in node like environments. #178

DonutEspresso opened this issue Feb 27, 2015 · 6 comments

Comments

@DonutEspresso
Copy link

We are building an isomorphic stack at Netflix, which means we require in modules via npm and use the same code in the browser.

We use html5shiv, but this module doesn't actually export anything when require'd in. In fact, because package.json is pointing to "dist/", and no dist/index.js file exists, calling require('html5shiv') will blow up.

To be fair, html5shiv has no contextual meaning in a Node like environment, but if it's published to npm it should work in any CommonJS like environment.

@aFarkas
Copy link
Owner

aFarkas commented Mar 2, 2015

It's a pretty special case. But I will do. A patch is also welcome, If you want to speed up. Note: AMD support is not needed.

@DonutEspresso
Copy link
Author

I agree it's a special case, though doing a proper exports will allow people to use Browserify as well without any extra config necessary. I'll try to whip up something if I get time this week. I know you said no AMD support needed, but most of the UMD boilerplates have it baked in. Any objections against using UMD?

@aFarkas
Copy link
Owner

aFarkas commented Mar 10, 2015

Sorry for taking so long. Please check whether this is what you need. (Note: we have for several reasons neglected support for AMD).

@DonutEspresso
Copy link
Author

Hey, thanks for putting this together! I haven't had time to take a stab at it yet from my end. I think your commit is a move in the right direction, though we'd need it to work in any CommonJS like environment. That means you should be able to do this from node CLI:

> require('html5shiv');
ReferenceError: document is not defined
    at Object.<anonymous> (/Users/aliu/Sandbox/shakti/html5shiv/dist/html5shiv.js:326:50)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at repl:1:1
    at REPLServer.defaultEval (repl.js:132:27)
    at bound (domain.js:254:14)

Currently, it doesn't work because it's assumed to be in a browser like environment with window and document globals. To make it truly environment agnostic would be a much larger undertaking, and would fundamentally change how html5shiv is exposed today. i.e., no automatic shimming or mutation of existing globals, consumers of html5shiv as a module would have to call an exposed API to start the shimming, etc.

However, I think it's unnecessary to do all that merely to make it Node-compatible given that it has no context in a server like environment. What we did in a fork was simply export an object stub when window is undefined. That way, none of the shimming code even attempts to execute and the above command would work in Node. Maybe something like:

(function(window, document) {
    if (typeof window === "undefined" && typeof exports !== "undefined") {
        exports = { stub: true,  };
    }
   ...
   ...
}(...));

What do you think?

@DonutEspresso
Copy link
Author

Another approach might be to simply return a stub object in node environments:

main: "./stub.js"

and a main entry point for Browserify:

browser: "./dist/html5shiv.js"

Given that you guys are exposed via npm and other CommonJS interfaces, these are the most likely entry points into the module. FYI, we already forked and return an empty stub as I suggested above, so there's no pressing need ATM. I'll leave it up to you and the other maintainers to decide what the best path forward is.

@ruchernchong
Copy link

Temporary add "main": "dist/html5shiv.js", to the package.json file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants