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

Update README.md to mention global variable #569

Conversation

turquoisemelon
Copy link

@turquoisemelon turquoisemelon commented Oct 11, 2017

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.

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.
@turquoisemelon turquoisemelon changed the title Update README.md Update README.md to mention global variable Oct 11, 2017
Copy link
Contributor

@mislav mislav left a 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.
Copy link
Contributor

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?

Copy link
Author

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

Copy link
Contributor

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')
Copy link
Contributor

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.

Copy link
Author

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.

@mislav mislav closed this in 1b721b5 Sep 4, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants