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

feat: pure esm #933

Closed
wants to merge 8 commits into from
Closed

feat: pure esm #933

wants to merge 8 commits into from

Conversation

jxom
Copy link
Member

@jxom jxom commented Sep 12, 2022

This PR converts wagmi from bundling both CJS + ESM, to bundling pure ESM.

I have published the packages to NPM as a snapshot release under the tag esm-only (npm i wagmi@esm-only).

This will probably affect tooling that is sensitive to importing ESM such as Jest and Remix. We might need to create some additional guides on how to use wagmi with these tools as we move to pure ESM.

@vercel
Copy link

vercel bot commented Sep 12, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
wagmi ✅ Ready (Inspect) Visit Preview Sep 13, 2022 at 5:06AM (UTC)

@changeset-bot
Copy link

changeset-bot bot commented Sep 12, 2022

🦋 Changeset detected

Latest commit: 22d6ad9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@wagmi/core Minor
wagmi Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jxom jxom changed the title wip: esm only feat: esm only Sep 12, 2022
@jxom jxom requested a review from tmm September 12, 2022 06:19
@jxom jxom changed the title feat: esm only feat: pure esm Sep 12, 2022
@@ -8,4 +8,5 @@ module.exports = {
serverBuildDirectory: 'build',
devServerPort: 8002,
ignoredRouteFiles: ['.*'],
serverDependenciesToBundle: [/^@?wagmi.*/, 'zustand/vanilla'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,949 @@
# Source: https://github.com/preconstruct/preconstruct/compare/main...jxom:preconstruct:tmm/esm-support
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source for the patch.

@@ -1,4 +1,4 @@
{
"main": "dist/wagmi-core-chains.cjs.js",
"module": "dist/wagmi-core-chains.esm.js"
"main": "dist/wagmi-core-chains.esm.js",
Copy link
Member Author

@jxom jxom Sep 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we don't even need these package.json's since we can just rely on package.json#exports, but we may need them for a couple of reasons:

  • Some tooling (Jest) don't respect the exports field, and
  • Setting moduleResolution to anything other than node16 or nodenext in tsconfig.json breaks type resolution without this. However, if we do set moduleResolution to one of those values, we need to append the .js extension to every relative import ¯_(ツ)_/¯. There are long running issues for this here and here.

@@ -1,5 +1,5 @@
import { BigNumber, logger } from 'ethers/lib/ethers'
import { Logger, formatUnits, isAddress } from 'ethers/lib/utils'
import { BigNumber, logger } from 'ethers/lib/ethers.js'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we aren't using ethers here? Should I move to that?

@tmm
Copy link
Member

tmm commented Oct 26, 2022

opening up PR with different approach shortly

@tmm tmm closed this Oct 26, 2022
@tmm tmm deleted the jxom/esm-only branch October 26, 2022 19:14
@tmm tmm mentioned this pull request Oct 26, 2022
1 task
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

Successfully merging this pull request may close these issues.

2 participants