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
Update README.md to mention global variable #569
Update README.md to mention global variable #569
Conversation
Proposing updating the installation section of the docs to mention polyfill global. There was an issue opened about this in the past: JakeChampion#539 . Please let me know what you think of my correction. I'd be more than happy to update it based on project maintainers' feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the improvements! I have a couple of questions.
|
||
```javascript | ||
import 'whatwg-fetch' | ||
``` | ||
|
||
In either case the polyfill sets up a global variable. The module does not export | ||
anything directly, so do not use the `import fetch from ...` form of the import statement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary to point out to clarify webpack instructions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it's a good idea to add an explanation as to why both approaches work. Especially after seeing the comments in this closed issue: #539
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry that this never got merged. The library went to some changes since then, and now it exports the fetch()
method and associated classes. I will update the README documentation to reflect that.
... or require the file: | ||
|
||
```javascript | ||
require('whatwg-fetch') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is require()
something specific to webpack? I'm not a webpack user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not specific to webpack but it's part of the common usage. I pointed it out as it is another way to load the package, in addition to the first one that was included in the docs.
Proposing updating the installation section of the docs to mention polyfill global. There was an issue opened about this in the past: #539 .
Please let me know what you think of my correction. I'd be more than happy to update it based on project maintainers' feedback.