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

Feature request: be a normal package #31

Closed
rektide opened this issue Aug 14, 2015 · 5 comments
Closed

Feature request: be a normal package #31

rektide opened this issue Aug 14, 2015 · 5 comments

Comments

@rektide
Copy link

rektide commented Aug 14, 2015

Munging the global state may be true to the spec, but it goes above and beyond how most people develop Javascript. Most front-end and back-end development is done with require() these days, and I think more people will be accustomed to using var fetch = require('isomoprhic-fetch') than would be comfortable using a global variable, much less than the number of people who understand what particular incantations you've invoked on behalf of the user to get the global state isomorphically munged like you do.

It's absolutely ok to provide a built in helper that munges the global state, adding fetch, but that should be a helper, not the default and only way to use this very fine package.

@tmcw
Copy link

tmcw commented Aug 14, 2015

I've seen before something like the es6-promise module that exposes a .polyfill() method to optionally write to globals.

@aredridel
Copy link

Yes, please! I won't use this module if it pollutes the global, but having the option would be wonderful.

@matthew-andrews
Copy link
Owner

I recognise the need for this but my team does not have this need and it would be a backwards incompatible change.

isomorphic-fetch is actually very, very, very small. All it does is merge together node-fetch and GitHub's Fetch polyfill so that the former is pulled in when you run require('isomorphic-fetch') on the server, and the latter on the client. isomorphic-fetch is mostly config, there's barely any code at all.

The reason why the isomorphic-fetch library exists is because in the past node-fetch didn't exist and this module used to provide the server side implementation. Now that node-fetch does exist, this module more or less blindly passes node-fetch through if you require('isomorphic-fetch') on the server.

I maintain isomorphic-fetch largely for backwards compatibility for old projects.

If I were to create a brand new project today I'd probably use the GitHub fetch polyfill on the client side directly (or with the polyfill service) and node-fetch directly in Node.

If someone wanted to fork isomorphic-fetch and got rid of the defining-itself-globally behaviour I would gladly link to that in the README.md of this module. I'd suggest calling it safe-isomorphic-fetch or perhaps universal-fetch.

I don't really want to maintain to different versions of isomorphic-fetch so for this issue I will say no. Sorry.

@aredridel
Copy link

Oh nice!

@paulmelnikow
Copy link
Contributor

I had a similar need to the original poster, and I assembled a proof of concept which mashed up the best of isomorphic-fetch and fetch-ponyfill. I got great results in Browserify, Webpack, and Node.

Rather than publish a new package right off the bat, I approached @qubyte to see if I could add what I needed to fetch-ponyfill

From qubyte/fetch-ponyfill#2 and qubyte/fetch-ponyfill#3:

I've got a proof of concept of a version of this which does not depend on brfs and is also agnostic of webpack. I adapted the clever approach you took here, of patching whatwg-fetch without vendoring in the source. But instead of doing it at compile time, I did that in a build step, and checked in the result.

It's a little bit gross to check in the generated code, but I like that it's a bundler-agnostic solution. Agnosticism is a good thing. An ecosystem of opinionated packages leads to JavaScript fatigue, which is something I'm feeling pretty heavily these days.

To me, checking in a generated CommonJS module seems like a reasonable compromise for bundler agnosticism, though I'm curious your thoughts.

I need a Node + Webpack ponyfill for my current project, but I haven't found one of those – hence building this POC – and I'd rather use something that is Browserify-compatible too. My team is trying out webpack, mostly for hot loading, but hasn't really settled on it in the long term. I know there are some hot-loading solutions for Browserify now. Really, just trying to stay agnostic.

Would you be interested in taking a look at it and seeing if you'd like to incorporate it into this project? If you'd like to incorporate it here, I can avoid publishing a new project (and adding marginally to everyone else's JS fatigue). If not, I certainly have no problem publishing it separately.

I'm pleased to say that has all been merged, so you can now use this in Node or the browser:

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

Or, provide a Promise implementation, to support older browsers and older Node environments:

var Promise = require('promise');
var fetch = require('fetch-ponyfill')({ Promise: Promise });

Hopefully you're still up for adding a link to the readme! I'll open a PR.

P.S. I did look around for other working solutions before implementing this. There is a package called universal-fetch, but it's a fork of this repo that supports IE8, and still a polyfill. And, by the way, thanks for the clear "wontfix" in this thread!

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

5 participants