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

fetch is not defined error due to missing fetch import in tableland-calls.ts #116

Closed
dtbuchholz opened this issue Aug 21, 2022 · 8 comments

Comments

@dtbuchholz
Copy link
Contributor

Describe the bug

I'm trying to simply use Tableland in an ES module, but I always get the error fetch is not defined, stemming from tableland-calls.js (using @tableland/sdk@3.1.0). I use node v18.7.0 and npm 8.15.0. My package.json has a type: module defined, and I successfully import fetch from "node-fetch" and use fetch elsewhere in my code. I try replacing my fetch import with the following 1-liner in my code (as recommended per the JS SDK docs for CJS/require), but it has no impact:

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

The only way to stop this error in my application is (usually) by reverting to CJS over ESM and then using the 1-liner/polyfill, which is not ideal. Thus, I'm assuming the issue must originate from Tableland, not my code.

After adding this 1-liner to tableland-calls.js in dist, the error is resolved. Upon digging into the SDK code a bit, it looks like the error stems from the fact tableland-calls.ts does not import node-fetch. If you import node-fetch into tableland-calls.ts and build, everything is fixed; no more fetch is not defined error.

Disclaimer: I know we use a modern version of Node, but I'm using Node v18 and still having this issue...so I'm not sure exactly why this issue is happening, to be honest. I would expect that I wouldn't have to import node-fetch with Node 18. Albeit, I'm not an expert on these intricacies. The fix described below resolves the error, and I've heard this problem pop up a few times from others, before. Not exactly sure if they were using CJS or ESM or what version of Node, though.

To Reproduce

  1. Create some Node.js ESM (using JS!) and install deps node-fetch as well as @tableland/sdk; call the file index.js.
  2. Import these into your index.js:
import fetch from "node-fetch"
import { connect } from "@tableland/sdk"
  1. Try to use the SDK's read query after connecting. You will get an error:
file:///Users/db/Tableland/projects/discord-table-bot/node_modules/@tableland/sdk/dist/esm/src/lib/tableland-calls.js:22
        const res = yield fetch(`${this.options.host}/rpc`, {
                    ^
ReferenceError: fetch is not defined
  1. Try to fix it with the 1-liner within index.js, and nothing is resolved.
  2. Manually add it in your node_modules/@tableland/sdk/dist/esm/src/lib/tableland-calls.js, and things work without errors:
const fetch = (...args) => import("node-fetch").then(({ default: fetch }) => fetch(...args))

Alternatives:

Here's the fix:

  1. In tableland-calls.ts, add @types/node-fetch:
npm i --save-dev @types/node-fetch --force
  1. Import node-fetch into tableland-calls.ts:
import fetch from 'node-fetch'
  1. Build. Now, the index.js file mentioned above works as expected without any messy node-fetch issues.

Note: in step (2) of the fix, if you don't --force, then ethers and siwe won't let you install, throwing this error:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR! 
npm ERR! While resolving: siwe@1.1.6
npm ERR! Found: ethers@5.6.9
npm ERR! node_modules/ethers
npm ERR!   ethers@"^5.6.9" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer ethers@"5.5.1" from siwe@1.1.6

Expected behavior:

Be able to use Tableland in a JS ESM without polyfills or workarounds for node-fetch.

Environment:

  • OS: Mac
  • @tableland/sdk 3.1.0
  • JS, not TS
  • node v18.7.0
  • npm 8.15.0
@dtbuchholz
Copy link
Contributor Author

I get the logic behind Node 18 not needing the node-fetch import. It's odd that fetch doesn't work for me natively since I use Node 18, but I can't use the SDK in an ESM and can only use it with CJS+{the 1-liner} because of this. For reference, Node 18 takes over Node 16 LTS on October 25. If others aren't facing this issue, then I'll have to do some more digging, but it's frustrating.

@dtbuchholz
Copy link
Contributor Author

dtbuchholz commented Aug 23, 2022

A member from Discord has noted a similar experience:
image

And also, running hardhat requires the current LTS version. In evm-tableland, it uses hardhat: 2.9.9, which doesn't support Node 18. For example, if you try and run the local-tableland script with Node 18, it'll throw something like:

Unsupported engine {
    package: 'hardhat@2.9.9',
    required: { node: '^12.0.0 || ^14.0.0 || ^16.0.0' },
    current: { node: 'v18.7.0', npm: '8.15.0' }
}

@carsonfarmer
Copy link
Member

If you follow the linked recommendation in the docs, are you able to avoid the above issue, even with an ESM build? This is linked from our docs, and suggests having an entry point which polyfills the fetch API: https://github.com/node-fetch/node-fetch#providing-global-access. There are some funny things with node-fetch between version 2 and version 3 (which is ESM only). I highly doubt we'll get anything that works smoothly between NodeJS and the browser without fetch polyfills at this early stage...

@joewagner
Copy link
Contributor

joewagner commented Aug 25, 2022

@dtbuchholz I installed the latest node, I think it's new today, v18.8.0 and removed the import fetch from "node-fetch" and globalThis.fetch = fetch lines from the gist you shared in #115.
I renamed your gist file to use .mjs file ending, since I don't have a package.json, and I'm able to create tables and read tables and everything is working.
If you update to v18.8.0 do things work?
Alternatively can you try the --experimental-fetch flag when you run node?

@dtbuchholz
Copy link
Contributor Author

Sorry for the delay. Gonna just close this out -- I think I just have something weird going on with my mac. I'm going to try and debug it or change to a new one that I have, and once that's resolved, I'll consider revisiting this if it's still an issue.

@tansanDOTeth
Copy link

I ran into this issue as well

@tansanDOTeth
Copy link

tansanDOTeth commented Nov 3, 2022

If you follow the linked recommendation in the docs, are you able to avoid the above issue, even with an ESM build?

Did I miss it in the readme? could you please share it here?

This is linked from our docs, and suggests having an entry point which polyfills the fetch API: https://github.com/node-fetch/node-fetch#providing-global-access.

I do understand we can do this. However, it just seems a bit incomplete to not have this included into the library if there is an dependency for it.

@dtbuchholz
Copy link
Contributor Author

@tansanDOTeth it's not in the README, but let me update that; it's the same link as the one noted. If you use Node v18, the issue is resolved (as of a few days ago, v18 is now LTS and v16 & v14 are maintenance: https://github.com/nodejs/Release).

This is a tough one. Having the SDK quietly polyfill fetch feels heavy handed and prone to problems, but we could be wrong here. As an example, when node-fetch went from v2 vs. v3, the SDK could no longer use it because of incompatibilities combining TypeScript, ESM, and Jest. An example issue: node-fetch/node-fetch#1265

On the flip side, this is the most common issue that people run into when using the SDK (including myself), which makes it challenging to ignore.

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

No branches or pull requests

4 participants