require('isomorphic-fetch/safe') to not set globals

[gobengo]

This pull allows users of the library to do

var fetch = require('isomorphic-fetch/safe');
require('assert')(typeof global.fetch === 'undefined');

'safe' requires do not result in any new global.fetch.

I'd like to use this library as a dependency in a lib I'm working on which, when run in the browser, should not touch global scope. This PR makes that use case possible without having to fork the lib.

Behavior of require('isomorphic-fetch') should remain unchanged (ie it will return the same thing and continue setting global.fetch on server and browser).

[matthew-andrews]

This would only work on the server, not the client…… and making things consistent between server + client is perhaps the main purpose of this repository 😄.

I don't understand why people want this…… isomorphic-fetch does almost nothing except for define globals.

If you want to do something in a more controlled way then I think you'd be better off using the underlying libraries that iso-fetch pulls in……

$ npm install --save node-fetch whatwg-fetch

Then in client side do:-

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

And on the server do:-

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

[gobengo]

It does work in the client... I used this fork in https://github.com/Livefyre/livefyre-geo-collection

"I don't understand why people want this……". I mentioned in the PR description: "I'd like to use this library as a dependency in a lib I'm working on which, when run in the browser, should not touch global scope. This PR makes that use case possible without having to fork the lib."

No sweat on closing it though. If anyone wants a version of this that doesn't create globals, use my fork. File an issue if you want me to rebase.

[matthew-andrews]

Hmm, thank you for replying with the extra details. I think I see what you mean now. That's tricky.

Normally for libraries like yours' I would try to encourage the consumer to bring their own polyfills. It's because of the reason why this library (or denodeify) don't ship with a Promise polyfill, which they depend on to actually work in all but the latest browsers.

But on the other hand I can see why for a component like yours' you would want to make developers' lives as easy as possible [whilst at the same time not polluting global scope].

I'm going to think about this for a few days. Thank you again.

[JakeChampion]

I would recommend caching this require call outside of the function and ensure it closes over it. Without this change, I believe you would be hitting the synchronous require with each fetch.

[billinghamj]

Presumably the main blocker to this is the fact that GitHub's implementation cannot be used without polluting the global scope. Due to the fact that they refuse to implement module support, this can't be changed.

[JakeChampion]

@gobengo Do you have a reference to the fact about Github refusing to implement module support? I was about to start work on creating a fetch polyfill which didn't shim by default (pollute globals) and was going to submit it as a patch to Github's fetch polyfill.

[billinghamj]

@JakeChampion JakeChampion/fetch#184 (comment)

[paulmelnikow]

@JakeChampion I'm curious, did you get around to doing that? If not, you might be interested in this.

I recently worked with @qubyte to implement isomorphism and bundler-agnosticism in fetch-ponyfill. It's a grand hack which processes Github's implementation in a build step, wrapping it in a closure and extracting it via thisArg. Works great in Node, Webpack, and Browserify.