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

.textConverted needs to install the package encoding separately #412

Closed
motss opened this issue Mar 4, 2018 · 9 comments
Closed

.textConverted needs to install the package encoding separately #412

motss opened this issue Mar 4, 2018 · 9 comments

Comments

@motss
Copy link

motss commented Mar 4, 2018

Why the package encoding is not part of node-fetch's dependencies to use .textConverted?

@TimothyGu
Copy link
Collaborator

It's intentional: we don't force people who don't use textConverted to install encoding. We should document this however.

@motss
Copy link
Author

motss commented Mar 5, 2018

Ya. A line or two stating this can be helpful! 👍

@TimothyGu
Copy link
Collaborator

@motss Would you be interested in creating a pull request? ☺️

@motss
Copy link
Author

motss commented Mar 6, 2018

@TimothyGu Sure thing. Will work on that probably on the weekend.

bitinn added a commit that referenced this issue Mar 9, 2018
I am trying to start a PR that fixes many small issues with current readme.

Specifically, these issues: #412, #399, #375, #368

Love to hear some comments (or provide cleaner code example for above issues!)
@peterbakonyi05
Copy link

peterbakonyi05 commented Apr 5, 2018

Please note that since encoding is not part of the dependencies by default, when using this package with webpack, without installing encoding, webpack will fail with the usual error message:

WARNING in ./node_modules/cross-fetch/node_modules/node-fetch/lib/index.es.js
Module not found: Error: Can't resolve 'encoding' in 'C:\git\halo.site\node_modules\cross-fetc
h\node_modules\node-fetch\

I think it would be nice to also add this to the README:
When using node-fetch with webpack. You either need to install encoding or add IgnorePlugin

new webpack.IgnorePlugin(/^encoding$/, /node-fetch/)

@itonics-tbeauvais
Copy link

Somehow, using the nodeExternals() for the externals option in webpack found here https://www.npmjs.com/package/webpack-node-externals

@motss
Copy link
Author

motss commented Aug 28, 2018

I thought this had been fixed by one of the PRs. I can update the doc real quick with the findings from @peterbakonyi05 as well.

@motss
Copy link
Author

motss commented Aug 28, 2018

I've added briefly in the doc. Please help review #511. Thank you.

@bitinn
Copy link
Collaborator

bitinn commented Nov 5, 2018

I have added guideline to readme and link to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants