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

Docs: don’t rely on global variable #41

Closed
rauschma opened this issue Oct 20, 2015 · 2 comments
Closed

Docs: don’t rely on global variable #41

rauschma opened this issue Oct 20, 2015 · 2 comments

Comments

@rauschma
Copy link

In the docs, I’d recommend the following way of using fetch():

var fetch = require('isomorphic-fetch');

I wouldn’t create a global variable in the Node.js code, either.

@matthew-andrews
Copy link
Owner

I've written up some notes on this here:-
#31 (comment)

The goal of this module is to provide the same API on the client as is provided on the server — it does that at the moment by defining fetch globally that's how I would like to encourage this library to be used (this is how the client side version of fetch works — and that's the API we're trying imitate here). If developers care about what's being defined globally I think they should just use node-fetch directly…

@gajus
Copy link

gajus commented Sep 14, 2016

The goal of this module is to provide the same API on the client as is provided on the server — it does that at the moment by defining fetch globally that's how I would like to encourage this library to be used (this is how the client side version of fetch works — and that's the API we're trying imitate here). If developers care about what's being defined globally I think they should just use node-fetch directly…

No library should write anything into a global state. This is in effect memory leaking. There is no way to dereference use of isomorphic-fetch after importing it. (Other than looking into the source code to tell which global variables you set.)

Conceptual principals aside, this is just bad design. In Node.js, you are promoting use of global variables. In Browser, you are tricking the environment to think that fetch is present. This would break an application that preforms feature checking to plan progressive enhancement.

On the bright side, you did address this in the documentation and suggested an alternative. Thumbs up for that. 👍

For ease-of-maintenance and backward-compatibility reasons, this library will always be a polyfill. As a "safe" alternative, which does not modify the global, consider fetch-ponyfill.

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