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

Can't update node-fetch #33

Closed
cwchristerw opened this issue Sep 1, 2021 · 13 comments · Fixed by #35
Closed

Can't update node-fetch #33

cwchristerw opened this issue Sep 1, 2021 · 13 comments · Fixed by #35
Assignees
Labels
bug Something isn't working dependencies Pull requests that update a dependency file help wanted Extra attention is needed
Milestone

Comments

@cwchristerw
Copy link
Member

No description provided.

@cwchristerw cwchristerw added help wanted Extra attention is needed dependencies Pull requests that update a dependency file labels Sep 1, 2021
@jimmywarting
Copy link

You can no longer use require to load node-fetch as it's now ESM only
you would have to switch to ESM also or use

const fetch = (...args) => import('node-fetch').then(({default: fetch}) => fetch(...args));

@cwchristerw cwchristerw linked a pull request Sep 2, 2021 that will close this issue
@cwchristerw
Copy link
Member Author

Then it will give this error in Javascript but typescript build doesnt fail.

Error [ERR_REQUIRE_ESM]: require() of ES Module /usr/src/app/node_modules/node-fetch/src/index.js from /usr/src/app/build/client/events/ready.js not supported.
Instead change the require of index.js in /usr/src/app/build/client/events/ready.js to a dynamic import() which is available in all CommonJS modules.

@cwchristerw
Copy link
Member Author

cwchristerw commented Sep 2, 2021

I wrote this earlier in node-fetch Discord:

I will put this into my Typescript file
const fetch = (...args) => import('node-fetch').then(({default: fetch}) => fetch(...args));

but it will be builded into this
const fetch = (...args) => Promise.resolve().then(() => __importStar(require('node-fetch'))).then(({ default: fetch }) => fetch(...args));

This causes ERR_REQUIRE_ESM, but if I manually edit file in build folder to const fetch = (...args) => import('node-fetch').then(({default: fetch}) => fetch(...args)); then it will work but give only
(node:18) ExperimentalWarning: stream/web is an experimental feature. This feature could change at any time
(Use node --trace-warnings ... to show where the warning was created)

Havent tested yet those possible solutions

@cwchristerw cwchristerw added the bug Something isn't working label Sep 3, 2021
@cwchristerw cwchristerw added this to the 0.3.10 milestone Sep 4, 2021
@cwchristerw cwchristerw linked a pull request Sep 6, 2021 that will close this issue
@cwchristerw
Copy link
Member Author

@cwchristerw
Copy link
Member Author

cwchristerw commented Sep 7, 2021

const _importDynamic = new Function('modulePath', 'return import(modulePath)');
const fetch = (...args) => _importDynamic('node-fetch').then(({default: fetch}) => fetch(...args));

const _importDynamic = new Function('modulePath', 'return import(modulePath)');
const fetch = (...args) => _importDynamic('node-fetch').then(({default: fetch}) => fetch(...args));

After npm build it's stays same.

@jimmywarting This is temporary fix to my issue

@cwchristerw
Copy link
Member Author

Is there a way to hide experimental feature warning @jimmywarting

(node:18) ExperimentalWarning: stream/web is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

@jimmywarting
Copy link

It is possible to run node with --no-warning flag
Also, if globalThis.ReadableStream is defined then it won't load stream/web that gives you the error

@cwchristerw
Copy link
Member Author

Is this safe way to import module if it works, can I push this to production bot when its time to release version 0.3.10.

@cwchristerw
Copy link
Member Author

I have planned #23 & #24, and they both require fetch to work properly 😄

@jimmywarting
Copy link

Grate idea about constructing a new function that don't get transformed

Will start recommend it in node-fetch also

@cwchristerw
Copy link
Member Author

Nice @jimmywarting 😄

@cwchristerw
Copy link
Member Author

cwchristerw commented Sep 7, 2021

I just looked for solution in microsoft/TypeScript#43329 and saw that there is example _importDynamic (microsoft/TypeScript#43329 (comment)) and then decided try it in combined with earlier #33 (comment)

@cwchristerw cwchristerw self-assigned this Sep 8, 2021
@cwchristerw cwchristerw modified the milestones: 0.3.10, 0.3.8 Sep 9, 2021
@Masterxilo
Copy link

Masterxilo commented Oct 21, 2021

Biggest headache of the day.

For anyone who does not have time to figure out how to get ESM to work with typescript and ts-node or whatever and just wants a browser-like fetch function fast, now, downgrade to node-fetch 2.x.x:

  "devDependencies": {
    "@types/node": "^16.10.3",
    "@types/node-fetch": "^2.0.0",
    "ts-node": "^10.2.1",
    "typescript": "^4.4.4"
  },
  "dependencies": {
    "node-fetch": "^2.0.0"
  }

and just import as

import fetch from 'node-fetch';

and get on with your life.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file help wanted Extra attention is needed
3 participants