Skip to content

Update README.md to mention global variable#569

Closed
turquoisemelon wants to merge 1 commit intoJakeChampion:masterfrom
turquoisemelon:turquoisemelon-patch-1
Closed

Update README.md to mention global variable#569
turquoisemelon wants to merge 1 commit intoJakeChampion:masterfrom
turquoisemelon:turquoisemelon-patch-1

Conversation

@turquoisemelon
Copy link
Copy Markdown

@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
Copy Markdown
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.

```

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants