aboutlo / ether-swr

Ether-SWR is a React hook that fetches Ethereum data. It streamlines the chores to keep the internal state of the Decentralized App (DApp), batches the RPC calls to an Ethereum node and cache the responses
MIT License
138 stars 12 forks source link

Add "exports" field to support Node.js ESM #10

Closed talentlessguy closed 3 years ago

talentlessguy commented 3 years ago

Hello again, I'm in process of writing next-eth, an Ethereum library for Next.js

It's a well-known problem that Next.js doesn't play with ESM well, but in recent version it got fixed. You can try it out with Next.js 11.0.2-canary.14 and use this config:

module.exports = {
  experimental: {
    esmExternals: 'loose'
  }
}

So there's a problem. My library depends on ether-swr. Next.js treats it as a CommonJS module because it has no "exports" field in package.json. And this results in this error:

SyntaxError: Named export 'etherJsFetcher' not found. The requested module 'ether-swr' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'ether-swr';
const { etherJsFetcher } = pkg;

the solution is to make this package mixed - e.g. have an "exports" declaration that points to CJS and ESM versions. Note that it will still work the same for old and new versions of Next.js

For more information, see this:

aboutlo commented 3 years ago

I see, I don't see why the test on build 14.x is failing. It works fine locally. Can you change in the main.yml file the matrix to: node-version: [12.x, 14.x, 16.x]? so we can trigger a re-run

aboutlo commented 3 years ago

also, I would consider this as fix not a feat 🙏

talentlessguy commented 3 years ago

@aboutlo oops sorry, will change commit message

talentlessguy commented 3 years ago

lol tests fail on node v14 for no reason...

aboutlo commented 3 years ago

I think it a kind of race condition. The events part is a bit fragile and I should rework it... but I have no bandwidth currently :/

aboutlo commented 3 years ago

Ok, green but for sure something is off there

talentlessguy commented 3 years ago

@aboutlo yay thanks! weird tbh that it randomly failed before but it's green now anyways so... it works

talentlessguy commented 3 years ago

Looks like "type": "module" is required for the package to work with ESM

or for files to have .mjs extension

see: https://github.com/vercel/next.js/issues/23725#issuecomment-886988737

aboutlo commented 3 years ago

I fear this need to rethink how the build is done. Perhaps changing the template to something that supports module correctly.