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

Make reqwest client/server agnostic #153

Closed
appsforartists opened this issue Sep 2, 2014 · 17 comments
Closed

Make reqwest client/server agnostic #153

appsforartists opened this issue Sep 2, 2014 · 17 comments

Comments

@appsforartists
Copy link

Right now, trying to use reqwest from Node results in this error:

> var reqwest = require("reqwest")
ReferenceError: window is not defined
    at ./node_modules/reqwest/reqwest.js:13:13
    at win (./node_modules/reqwest/reqwest.js:8:72)
    at Object.<anonymous> (./node_modules/reqwest/reqwest.js:11:2)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:349:32)
    at Function.Module._load (module.js:305:12)
    at Module.require (module.js:357:17)
    at require (module.js:373:17)
    at repl:1:15

Given that there are projects to provide the XHR interface in Node, making reqwest isomorphic should be as simple as abstracting out the implicit use of globals like window and document.

@appsforartists
Copy link
Author

Started working on it here:

https://github.com/appsforartists/reqwest

@alexaivars
Copy link

+1

@markbahnman
Copy link

Ran into this same issue when prerendering a react page that used reqwest, being able to be used in node would be a huge plus.

@appsforartists
Copy link
Author

@markbahnman The fork I linked above has been working for me.

@ded
Copy link
Owner

ded commented Oct 17, 2014

@appsforartists @alexaivars @markbahnman
this is an interesting issue given I've always used request within Node since it solves a plethora of other problems. Given Node and the browser are two completely different beasts, it would bloat this library to no end. Therefore, when I'm working on the frontend, I use reqwest, and when in node, I use request.

Go ahead and look at the source for https://github.com/mikeal/request and you'll see it solves a number of other problems that this library doesn't even attempt to address. And visa/versa.

@appsforartists
Copy link
Author

@ded I haven't looked through request much, but if xhr2 continues to work as-advertised, that should polyfill all you need to worry about for the HTTP layer in Node. Then, it's just a matter of being careful about not-presuming that browser properties are global. As you can see, the footprint of appsforartists@796d511 is pretty damn small.

It's your library. You've spent a lot of more time than I have thinking about these things, and I'm certainly not going to tell you what to support, but are you amenable to something like appsforartists@796d511 to enable reqwest to be used in isomorphic apps (if the node-implementation can be offloaded to a polyfill like xhr2)?

The apps I'm working on nowadays use identical code on the client and server, so using two different libraries for data is really unappealing. If we could make reqwest isomorphic without bloating or wholesale rewriting, that would be awesome!


Also interesting to note, @mjackson's mach is aiming to be the end-all http library for JS, equally adept at the client and server. However, I don't think the client story has been fleshed out yet.

@blainekasten
Copy link

+1 for agnosticism. (in this context)

@raysuelzer
Copy link

+1 right now, I can't use the library with reactjs,net without a hassle.

@raysuelzer
Copy link

@appsforartists could you put in a pull request for this issue? Isomorphic apps are becoming more and more popular, so being able to include this dependency in webpack is pretty important. I don't want to have to hack together two different versions of my javascript more than I need to.

@appsforartists
Copy link
Author

@raysuelzer sure. FWIW, I use mach now, which is agnostic. If you're feeling future-proof, there's also fetch.

@appsforartists
Copy link
Author

@raysuelzer I don't know if @ded is interested, but there's now a PR open at #185.

@blainekasten
Copy link

@appsforartists thanks for doing that! Hopefully @ded is interested.

@luandro
Copy link

luandro commented May 9, 2015

+1 for reqwest in isomorphic apps

1 similar comment
@yicone
Copy link

yicone commented Jul 5, 2015

+1 for reqwest in isomorphic apps

@iamrandys
Copy link

+1

@ded
Copy link
Owner

ded commented Jul 13, 2015

alright this is officially on the radar. i'll have a look at that open PR as well!

@ded
Copy link
Owner

ded commented Jul 14, 2015

@appsforartists @blainekasten @raysuelzer @luandro @yicone

Thanks tons for being patient on this! Given the high level of interest on this, and taking into account the history of this project, we'll have to consider a solution that continues to support the original purpose (the browser) and somehow add a peer dependency for Node.js (like the xhr2 library) that doesn't conflict with other client package managers (like Browserify, RequireJS, or Ender).

In the meantime, I'll try and have a play with a solution

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

9 participants