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

"fetch" is not defined #821

Closed
yaodingyd opened this issue Mar 15, 2017 · 22 comments

Comments

@yaodingyd
Copy link

commented Mar 15, 2017

When I use fetch, a polyfill is imported like this import 'whatwg-fetch', and "fetch" is not defined error would show up.

I know that I could use "global" to prevent this error, but should these been considered in standard for these common polyfill?

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2017

@yaodingyd

This comment has been minimized.

Copy link
Author

commented Mar 15, 2017

@yoshuawuyts Thanks! I should have thought of using window before.

@yaodingyd yaodingyd closed this Mar 15, 2017

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2017

@indiesquidge

This comment has been minimized.

Copy link

commented Mar 18, 2017

@yoshuawuyts, how does this take into account server-side use of fetch? There is no window object in Node. I know I could just use /* global fetch */ for the files where I'm using fetch, but this seems like a bit of a nuisance.

@falmar

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2017

It may be just be a import from a package like node-fetch? it would be inside your file scope as import so no need for global.

@feross

This comment has been minimized.

Copy link
Member

commented Mar 18, 2017

Exactly what @falmar said. In node, you just do:

const fetch = require('node-fetch')
fetch('https://github.com/', ...)
@indiesquidge

This comment has been minimized.

Copy link

commented Mar 18, 2017

Hmm, aren't things like isomorphic-fetch meant to be imported as a global (import 'isometric-fetch'), not as a default or named import?

@feross

This comment has been minimized.

Copy link
Member

commented Mar 18, 2017

Not a good idea, IMO. As a rule, modules shouldn't set global variables. A better approach would be to import fetch in both node and the browser environment. (In the browser, it could simply return window.fetch.)

If you still want to use isomorphic-fetch, I recommend documenting its behavior with a comment:

import "isometric-fetch" /* global fetch */
@indiesquidge

This comment has been minimized.

Copy link

commented Mar 18, 2017

Not a good idea as in "I don't recommend using isomorphic-fetch", or not a good idea as in "use it like const fetch = require('isomorphic-fetch')"?

I thought isomorphic-fetch was meant to act as a polyfill and thus needed to be global, no? Perhaps it can be import as a default import? I guess I just hadn't seen it used that way before.

@indiesquidge

This comment has been minimized.

Copy link

commented Mar 18, 2017

import "isomorphic-fetch" /* global fetch */

I'm currently doing it this way, but it did feel kind of icky, and I was hoping there was a better solution. However, I'm not sure I fully understand what you're suggesting as an alternative. Thanks for replying so quickly to all of this though!

@LinusU

This comment has been minimized.

Copy link
Member

commented Mar 18, 2017

isomorphic-fetch exports the function in addition to patching the global scope. The following should work great:

const fetch = require('isomorphic-fetch')

fetch('http://example.com')
  .then(res => res.text())
  .then(text => console.log(text))
@indiesquidge

This comment has been minimized.

Copy link

commented Mar 19, 2017

@LinusU, are you sure of this? I haven't seen examples of it used as a ponyfill like you are exampling, and there are lots of issues and PRs which seem to hint otherwise—the README itself warns that the library "adds fetch as a global so that its API is consistent between client and server."

@indiesquidge

This comment has been minimized.

Copy link

commented Mar 19, 2017

Apologies, I mis-read your comment and see now that you say "in addition to patching the global scope". However, if isomorphic-fetch is already patching the global scope, what is the use of doing something like const fetch = require('isomorphic-fetch')? IMO, having something like require('isomorphic-fetch') /* global fetch */ is far more honest about what is really going on.

@falmar

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2017

I highly recommend you to use it like @LinusU in his snippet. It is way more declarative and you will know exactly what is going on. I do expect that isomorphic-fetch will inject fetch into the global scope, but what if there is something that change it and wont work as expected. I rather import it and use that way, maybe when you are on web browser environment it gives you the actual fetch from window and if it is another environment it will supply you with their custom isomorphic-fetch

@indiesquidge

This comment has been minimized.

Copy link

commented Mar 19, 2017

It is way more declarative and you will know exactly what is going on

How? The library is replacing the global fetch...

but what if there is something that change it and wont work as expected

I'm not sure I fully understand what you're getting at here, but I think you may be misunderstanding isomoprhic-fetch. 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.

@feross

This comment has been minimized.

Copy link
Member

commented Mar 19, 2017

How? The library is replacing the global fetch...

I think the point is that the global fetch could be replaced by another pollyfill that executes later, in another package. By explicitly depending on your local fetch variable, you guarantee that your code will always get the fetch implementation that it is expecting.

@indiesquidge

This comment has been minimized.

Copy link

commented Mar 19, 2017

I think the point is that the global fetch could be replaced by another pollyfill that executes later, in another package.

I'm pretty confused by this. Wouldn't the global fetch just be set by isomorphic-fetch here? Why would it change? Are people including multiple polyfills in a single project?

@feross

This comment has been minimized.

Copy link
Member

commented Mar 19, 2017

Why would it change? Are people including multiple polyfills in a single project?

In an ideal world, no one would include multiple polyfills in a single project. However, if one of the packages you're using includes isomorphic-fetch or another implementation that also tries to overwrite the global, then the order these run in will affect the outcome.

It's not that complicated: packages should never write globals. If you, as a top-level user who is making an app, want to polyfill fetch, then you should do:

if (typeof window.fetch !== 'function') {
  window.fetch = require('some-fetch-implementation')
}

That said, isomorphic-fetch seems to otherwise be a good package, so it's totally fine if you want to use it. Just be explicit about where fetch is coming from to satisfy standard. To do that, you have two options:

  1. Use it directly (best)
const fetch = require('isomorphic-fetch')
fetch('http://example.com')
  1. Use the global
require('isomorphic-fetch') /* global fetch */
fetch('http://example.com')

This is all documented in the readme here: http://standardjs.com/#i-use-a-library-that-pollutes-the-global-namespace-how-do-i-prevent-variable-is-not-defined-errors

@indiesquidge

This comment has been minimized.

Copy link

commented Mar 19, 2017

Ah, that makes perfect sense, @feross. I really appreciate you taking the time to explain this, as I know you likely have many other things competing for your attention. I agree 100% with what you have laid out now that is has been explained in more detail. (Apologies for somehow missing that section in the README.)

Thanks again.

@feross

This comment has been minimized.

Copy link
Member

commented Mar 19, 2017

No problem!

@karnavkumar

This comment has been minimized.

Copy link

commented Mar 21, 2018

use this global.fetch = require('node-fetch')

@kranthi1996

This comment has been minimized.

Copy link

commented Apr 10, 2018

npm install --save isomorphic-fetch es6-promise

const fetch = require('isomorphic-fetch')

its working fine

@lock lock bot locked as resolved and limited conversation to collaborators Jul 9, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
8 participants
You can’t perform that action at this time.