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

Node support #2

Closed
paulmelnikow opened this issue Apr 30, 2016 · 4 comments
Closed

Node support #2

paulmelnikow opened this issue Apr 30, 2016 · 4 comments

Comments

@paulmelnikow
Copy link

I love this package. I use it with Browserify to ship code for an embeddable component, which developers can embed into their web pages, and appreciate that it doesn't manipulate globals to do its work.

Now I'm working on another project, and I'm having trouble. I'm writing an HTTP client, using fetch, which I want to ship via Browserify, and also test outside the browser, using mocha and node.

isomorphic-fetch has a solution for this, which appears to use the browser keyword in package.json. In the node environment, it polyfills global.fetch with node-fetch. I haven't tested it myself, but it seems to work.

Of course, I don't want a polyfill.

So my question is, would you be interested in adding support for this use case – code which needs to run both in Browserify and Node? If you are, I'd be happy to work up a PR.

@paulmelnikow
Copy link
Author

paulmelnikow commented Apr 30, 2016

var fetch = require('fetch-ponyfill')({ Promise: Promise });
/Users/pnm/code/blue-frontend/node_modules/fetch-ponyfill/index.js:12
    var XMLHttpRequest = options && options.XMLHttpRequest || window.XMLHttpRequest;
                                                              ^

ReferenceError: window is not defined
    at fetchPonyfill (/Users/pnm/code/blue-frontend/node_modules/fetch-ponyfill/index.js:12:63)
    at Object.<anonymous> (/Users/pnm/code/blue-frontend/test.js:1:100)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Function.Module.runMain (module.js:447:10)
    at startup (node.js:141:18)
    at node.js:933:3

I could work around this by providing an XHR implementation for Node, but I think a much better solution is to use node-fetch, which relies on native APIs.

Also, providing an XHR implementation wouldn't dodge the problem. I would want that implementation when I use fetch-ponyfill in node, but would not want it in the browser bundle.

@qubyte
Copy link
Owner

qubyte commented Apr 30, 2016

If you can get this module to detect when it's in Node, I wouldn't mind a conditional require for node-fetch. My worry would be that browserify would see that require and attempt to bundle node-fetch. If you can avoid that, then I'd certainly consider a pull request.

@paulmelnikow
Copy link
Author

Cool, thanks for the quick response! I opened a PR. Let me know what you think!

@paulmelnikow
Copy link
Author

I also have some tests, which run automatically in the Node environment, and manually in the browser environment. I imagine you have some kind of test workflow in place already. If you'd like these tests checked in, I'm happy to open a subsequent PR for them, or add them to this PR.

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

2 participants