blocknative / web3-onboard

Client library to onboard users to web3 apps
https://onboard.blocknative.com/
MIT License
844 stars 502 forks source link

[Bug]: global is not defined with walletconnect - w/ SvelteKit + Vite + pnpm #1568

Closed beeb closed 1 year ago

beeb commented 1 year ago

Current Behavior

When clicking the WalletConnect option, an error appears: global is not defined

Expected Behavior

The WalletConnect QR code should appear

Steps To Reproduce

  1. Clone this repo: https://github.com/beeb/lp-rescue-frontend
  2. pnpm run build
  3. pnpm run preview
  4. Open preview URL in browser
  5. Click Connect button in top right
  6. Click WalletConnect

What package is effected by this issue?

@web3-onboard/walletconnect

Is this a build or a runtime issue?

Runtime

Package Version

@web3-onboard/walletconnect: 2.2.2

Node Version

16.19

What browsers are you seeing the problem on?

Chrome

Relevant log output

No response

Anything else?

image

Here is where the walletconnect module is defined https://github.com/beeb/lp-rescue-frontend/blob/c1acad87df54292b622e6c2f2e634f423b89e64a/src/lib/constants.ts#L72-L94

Sanity Check

Adamj1232 commented 1 year ago

Hey @beeb! It looks like your svelte/vite config is missing some configurations - docs around this can be found here - https://onboard.blocknative.com/docs/modules/core#sveltekit Let me know if the issue still persists after updating your config

beeb commented 1 year ago

Hi @Adamj1232 . I think the documentation is outdated, since there is no vite item in the svelte config anymore. Nowadays this would probably go into the vite.config.ts file. Likewise, detection of the dev environment should probably happen with import { dev } from '$app/environment'. The problem is that this bug appears in production too, so the polyfill probably wouldn't help here.

And then for the rollupOptions the following is not a valid argument according to typescript: nodePolyfills({ crypto: true, http: true }):

export interface NodePolyfillsOptions {
    baseDir?: string;
    sourceMap?: RollupInjectOptions['sourceMap'];
    include?: Array<string | RegExp> | string | RegExp | null;
    exclude?: Array<string | RegExp> | string | RegExp | null;
}
beeb commented 1 year ago

I went forward anyway with the config and I get the following build error:

[commonjs--resolver] Could not load crypto-browserify: ENOENT: no such file or directory, open 'crypto-browserify'
file: /home/.../lp-rescue-frontend/node_modules/.pnpm/@unstoppabledomains+resolution@8.5.0/node_modules/@unstoppabledomains/resolution/build/index.js
error during build:
Error: Could not load crypto-browserify: ENOENT: no such file or directory, open 'crypto-browserify'
 ELIFECYCLE  Command failed with exit code 1.

The build was happening fine before I made the changes described in the docs.

beeb commented 1 year ago

I found this resource: https://github.com/blocknative/web3-onboard/blob/main/examples/with-sveltekit/vite.config.ts

That way I can compile successfully. But even with those changes I still get the same error.

beeb commented 1 year ago

Ok I took the official example linked above, updated all the dependencies to latest (using pnpm update), and then tried to run pnpm run build.

First few times it complained because @web3-onboard/injected-wallets and @web3-onboard/core were missing from the dependencies (they are not in the package.json), so I had to add them (pnpm add -D ...). After that it compiled but the bug I'm describing is also happening in the official example (global is not defined).

beeb commented 1 year ago

For your convenience I published the reproduction here, based on the official example:

https://github.com/beeb/repro-walletconnect

Adamj1232 commented 1 year ago

@beeb I can confirm this is an issue with walletconnect v1, v2 seems to be working as expected. Will have a v1 fix shortly.

Adamj1232 commented 1 year ago

@beeb the vite.config.js I was able to get working within the example project looks like:

import { sveltekit } from '@sveltejs/kit/vite'
import inject from '@rollup/plugin-inject'

import type { UserConfig } from 'vite'
import nodePolyfills from 'rollup-plugin-polyfill-node'

const MODE = process.env.NODE_ENV
const development = MODE === 'development'

/** @type {import('@sveltejs/kit').Config} */

const config: UserConfig = {
  plugins: [
    sveltekit(),
    development &&
      nodePolyfills({
        include: ['node_modules/**/*.js', new RegExp('node_modules/.vite/.*js'), 'http', 'crypto']
      })
  ],
  resolve: {
    alias: {
      crypto: 'crypto-browserify',
      stream: 'stream-browserify',
      assert: 'assert'
    }
  },
  build: {
    rollupOptions: {
      external: ['@web3-onboard/*'],
      plugins: [nodePolyfills({ include: ['crypto', 'http'] }), inject({ Buffer: ['Buffer', 'Buffer'] })]
    },
    commonjsOptions: {
      transformMixedEsModules: true
    }
  },
  optimizeDeps: {
    exclude: ['@ethersproject/hash', 'wrtc', 'http'],
    include: [
      '@web3-onboard/core',
      '@web3-onboard/sequence',
      'js-sha3',
      '@ethersproject/bignumber'
    ],
    esbuildOptions: {
      // Node.js global to browser globalThis
      define: {
        global: 'globalThis'
      },
    }
  },
  define: {
    global: 'window'
  }
}

export default config

Can you test on your end and see if this fixes WalletConnect v1(v2 works as well with this)

Adamj1232 commented 1 year ago

I have a branch working from the https://github.com/beeb/repro-walletconnect repo but cannot push because of privileges...if you want to give me access I can push up this example. I used the config above, removed the define.global and added updates the package.json to be

{
    "name": "lp-rescue-frontend",
    "version": "0.3.0",
    "private": true,
    "scripts": {
        "dev": "vite dev",
        "build": "vite build",
        "preview": "vite preview",
        "check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
        "check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
        "lint": "prettier --plugin-search-dir . --check . && eslint .",
        "format": "prettier --plugin-search-dir . --write ."
    },
    "devDependencies": {
        "@esbuild-plugins/node-globals-polyfill": "^0.2.3",
        "@iconify-json/ri": "^1.1.4",
        "@poppanator/sveltekit-svg": "^2.1.2",
        "@sveltejs/adapter-auto": "^1.0.2",
        "@sveltejs/kit": "^1.3.7",
        "@tailwindcss/typography": "^0.5.9",
        "@typescript-eslint/eslint-plugin": "^5.50.0",
        "@typescript-eslint/parser": "^5.50.0",
        "@web3-onboard/core": "^2.14.1",
        "@web3-onboard/injected-wallets": "^2.6.2",
        "@web3-onboard/walletconnect": "^2.2.2",
        "autoprefixer": "^10.4.13",
        "daisyui": "^2.50.0",
        "eslint": "^8.33.0",
        "eslint-config-prettier": "^8.6.0",
        "eslint-plugin-svelte3": "^4.0.0",
        "ethers": "^5.7.2",
        "postcss": "^8.4.21",
        "prettier": "^2.8.3",
        "prettier-plugin-svelte": "^2.9.0",
        "prettier-plugin-tailwindcss": "^0.2.2",
        "rollup-plugin-polyfill-node": "^0.12.0",
        "svelte": "^3.55.1",
        "svelte-check": "^2.10.3",
        "svelte-ethers-store": "^2.5.9",
        "svelte-notifications": "^0.9.98",
        "svelte-preprocess": "^4.10.7",
        "tailwindcss": "^3.2.4",
        "theme-change": "^2.3.0",
        "tslib": "^2.5.0",
        "typescript": "^4.9.5",
        "unplugin-icons": "^0.14.15",
        "vest": "^4.6.8",
        "vite": "^4.0.4"
    },
  "dependencies": {
    "@web3-onboard/core": "^2.15.4",
    "@web3-onboard/injected-wallets": "^2.8.1",
    "@web3-onboard/walletconnect": "^2.3.1",
    "buffer": "^6.0.3"
  },
    "type": "module"
}
Adamj1232 commented 1 year ago

I also added this:

    <script>
        /**
         * this is a hack for error: global is not defined
         */
        var global = global || window
    </script>

to app.html

Adamj1232 commented 1 year ago

I created a PR to get the the Web3-Onboard local working that you can see here: https://github.com/blocknative/web3-onboard/pull/1569 I havent had time to dig into why in your projects case that script needed to be added to the html. Could be that our example is use next versions of sveltekit. If you happen to find out please post it here.

beeb commented 1 year ago

if you want to give me access I can push up this example.

Done! Thanks for looking into this. I will try to see what are the minimal changes I need to make so that it works with v1. I'm pretty sure some of the configuration items should not be needed.

beeb commented 1 year ago

I used the config above, removed the define.global and added updates the package.json to be

Did you try with pnpm? Because I'm trying to reproduce your changes but I hit so many errors... I'll wait until you publish your branch and then test again.

beeb commented 1 year ago

@Adamj1232 I also noticed in my repro example that the pnpm run dev command doesn't work at all (some error with RxJS). I suggest we try to make this simple example work with latest sveltekit, pnpm, and the minimal amount of additional stuff.

Adamj1232 commented 1 year ago

@beeb pushed a working example to vite_config_working branch. Give er a look and let me know if it is working for you...for my testing I was using either npm(your project) or yarn (web3onboard local) I am not familiar with pnpm and dont really have the cycles to spend on it at this moment.

beeb commented 1 year ago

I'm sorry @Adamj1232 but your branch in the repro repo doesn't work for me, even with regular npm. Also, why did you switch to walletconnect v2? The goal was to fix v1.

Dev mode is broken with pnpm. But regardless, npm run build and pnpm run build both fail completely.

[vite]: Rollup failed to resolve import "Buffer" from "/home/valentin/temp/repro-walletconnect/node_modules/@walletconnect/encoding/dist/esm/index.js".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`

There are for some reason 2 polyfill libraries loaded, I'm pretty sure some of the stuff that was added doesn't contribute to solving the problem.

beeb commented 1 year ago

Ok, so I had a deeper look and approached the problem methodically. The workaround is now available in main here https://github.com/beeb/repro-walletconnect

PNPM

First observation: pnpm is not supported. The dev server (pnpm run dev) throws an error related to rxjs on SSR:

Internal server error: exports is not defined
      at eval (/node_modules/.pnpm/rxjs@7.8.0/node_modules/rxjs/dist/cjs/operators/index.js:4:23)
      at instantiateModule (file:///home/valentin/temp/repro-walletconnect/node_modules/.pnpm/vite@4.1.4/node_modules/vite/dist/node/chunks/dep-ca21228b.js:52420:15)
ReferenceError: exports is not defined
    at eval (/node_modules/.pnpm/rxjs@7.8.0/node_modules/rxjs/dist/cjs/operators/index.js:4:23)
    at instantiateModule (file:///home/valentin/temp/repro-walletconnect/node_modules/.pnpm/vite@4.1.4/node_modules/vite/dist/node/chunks/dep-ca21228b.js:52420:15)

When the dependencies are installed with npm the dev server works fine.

Buffer polyfill

It seems some component or dependency requires Node's Buffer. To polyfill this, the simplest way I could find was to install the buffer package and include the following in web3-onboard.ts:

import { Buffer } from 'buffer'
globalThis.Buffer = Buffer

Global is not defined

Finally, since global is not defined, I added the following to the app.html, according to your suggestion:

<script>
      var global = global || window
</script>

That's it

Nothing more, nothing less is needed. I didn't change the vite.config.ts file at all.

It's very annoying that this library is not compatible with pnpm, since it's much much faster and space-efficient than npm and has grown in popularity a lot.

Adamj1232 commented 1 year ago

@beeb Im glad you were able to resolve your issue which seems to be a problem with pnpm. You could consider digging into what is happening on their end as Web3-onboard works with npm and yarn without problem. I will make sure this is noted in our docs, thanks.

beeb commented 1 year ago

@Adamj1232 the main difference with pnpm is that it symlinks the dependencies into node_modules from a central package store, so you avoid to have duplicate packages stored on disk everywhere. Some packages use the node_modules folder in weird ways which conflicts with how pnpm does it. That's my best guess, but I don't have the resources to go dig deeper. Although I've been using pnpm for a year now and this is the only time I had to switch npm, so something is definitely not perfect in web3-onboard or one of its dependencies. Also, until recently it was working fine with pnpm and it broke during a minor version update of the packages.

Adamj1232 commented 1 year ago

@beeb interesting, thanks for sharing. It is challenging supporting many downstream deps along with newer projects like pnpm and the huge number of front end frameworks, bundlers and compilers as everything evolves. We are an open source project and we are free to use and contribute to so we rely on community members such as yourself to help with edge cases.

beeb commented 1 year ago

@Adamj1232 I'm very thankful for the contributors to this project that allowed me to kickstart my dApp. Open-source is wonderful but can be challenging sometimes, especially when you find yourself in dependency hell. I agree that the current landscape of web frontend is extremely challenging.

I really just wish there was a nice clean solution for connecting wallets that doesn't have thousands of dependencies and that is "more" framework agnostic.

Thanks for your help!

multiplehats commented 1 year ago

Same issue here with pnpm. Unforutnately I can't drop pnpm, so I'll wait until it's supported. I don't know enough about these tools to make a meaningful contribution. But this exact issue what I'm facing too: https://github.com/blocknative/web3-onboard/issues/1568#issuecomment-1463963462