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

[Bug]: using onboard along with Vite #794

Closed
imsys opened this issue Jan 23, 2022 · 11 comments
Closed

[Bug]: using onboard along with Vite #794

imsys opened this issue Jan 23, 2022 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@imsys
Copy link

imsys commented Jan 23, 2022

Current Behavior

I open the page and get an error thrown, it happens on the initialization of Onboard

Onboard({
    networkId: 1,
    walletSelect: {
        wallets: [
            { walletName: 'metamask' },
            { walletName: "coinbase" }
        ]
    }
});

### Expected Behavior

No errors

### Steps To Reproduce

In this environment:

```json
"type": "module",
"scripts": {
	"dev": "svelte-kit dev",
	"build": "svelte-kit build"
},
"devDependencies": {
	"@sveltejs/adapter-auto": "next",
	"@sveltejs/adapter-node": "^1.0.0-next.66",
	"@sveltejs/adapter-static": "^1.0.0-next.26",
	"@sveltejs/kit": "^1.0.0-next.240",
	"@typescript-eslint/eslint-plugin": "^4.31.1",
	"@typescript-eslint/parser": "^4.31.1",
	"autoprefixer": "^10.4.2",
	"eslint": "^7.32.0",
	"eslint-config-prettier": "^8.3.0",
	"eslint-plugin-svelte3": "^3.2.1",
	"postcss": "^8.4.5",
	"postcss-load-config": "^3.1.1",
	"prettier": "^2.4.1",
	"prettier-plugin-svelte": "^2.4.0",
	"rollup-plugin-polyfill-node": "^0.8.0",
	"sass": "^1.46.0",
	"svelte": "^3.46.2",
	"svelte-check": "^2.3.0",
	"svelte-preprocess": "^4.10.2",
	"tslib": "^2.3.1",
	"typescript": "^4.5.5"
},
"dependencies": {
	"bnc-onboard": "^1.37.1",
	"ethers": "^5.5.3",
	"web3": "^1.7.0"
}

I run npm run dev, or also in the build npm run build and I get the errors mentioned bellow.

Idk if the problem is SvelteKit, the node version, typescript or what. But from what I tried to understand, it seems to think that I'm using a Gnosis Safe wallet, when I'm not.

Onboard Version

1.37.1

Node Version

v17.3.1

What browsers are you seeing the problem on?

Firefox, Chrome

Relevant log output

Uncaught (in promise) TypeError: _context.t2 is not a constructor
    _callee$ gnosis-3bdbc36a.js:39
    Babel 10
    1 onboard-8f8ec111.js:9014
    promise callback*init$1 onboard-8f8ec111.js:9012
    WalletClass Wallet.ts:70
    instance Header.svelte:30
    run index.mjs:18
    mount_component index.mjs:1802
    flush index.mjs:1067
    init index.mjs:1894
    Root root.svelte:920
    createProxiedComponent svelte-hooks.js:266
    ProxyComponent proxy.js:239
    Proxy<Root> proxy.js:346
    _init start.js:885
    start start.js:738
    start start.js:1373
    <anonymous> (index):16

Anything else?

Screenshot_20220123_150032

Screenshot_20220123_155757

Screenshot_20220123_151453

Screenshot_20220123_151813

Screenshot_20220123_151902

Screenshot_20220123_031810

I also tried to run that import while in the debug:
Screenshot_20220123_151244

Maybe the bundler should be converting that import to an importable path? Is that the expected behavior, or is that injected and accessible when someone is using Gnosis Safe?

After doing all this debug, I'm inclined to think that problem is my bundler, as @gnosis.pm/safe-apps-sdk is in onboard dependencies.

@imsys imsys added the bug Something isn't working label Jan 23, 2022
@imsys
Copy link
Author

imsys commented Jan 23, 2022

I was able to get it working on the build for production, by using @rollup/plugin-node-resolve, I still need to get it working in development mode and I will share my config file. Then I think it would be nice to have this information included in the documentation.
I know @theurgi had a similar problem at #762, but Idk why that was not enough to I get my environment working.

@imsys imsys changed the title [Bug]: gnosis false positive detection? TypeError: _context.t2 is not a constructor [Bug]: using onboard along with rollup Jan 23, 2022
@imsys
Copy link
Author

imsys commented Jan 24, 2022

I was creating a public repository to share, and it seems @rollup/plugin-node-resolve was not required. Maybe it was the sveltekit version.
But anyway, the problem is just the gnosis error message in development. Metamask wallet connection is working, and apparently WalletConnect too. I haven't done much tests yet to know if there is any other error.

Anyway, the test repo is located at: https://github.com/imsys/sveltekit-onboard-test

@imsys imsys changed the title [Bug]: using onboard along with rollup [Bug]: using onboard along with Vite Jan 25, 2022
@imsys
Copy link
Author

imsys commented Jan 25, 2022

I open an issue at vitejs/vite#6632 referring to this.

@taylorjdawson
Copy link
Contributor

Hey @imsys what is the latest state of this issue. Seems like everything is working okay for you now?

@imsys
Copy link
Author

imsys commented Jan 26, 2022

This package is using RollUp to provide two versions: a CJS version and a ESM version.
The problem is that the ESM version has a CJS package within it (gnosis), and Vite was not expecting that to happen.
All the current updates are at vitejs/vite#6632

There are two ways to this to be fixed:

  • RollUp, to convert the CJS packages imports to a friendly way to be imported by ESM.
  • or Vite could verify for CJS imports inside ESM modules.

I'm just wondering if RollUp could have already fixed this.
Onboard is using RollUp version "^1.27.5"
Latest versions are:
"1.32.1"
"2.66.1"

@imsys
Copy link
Author

imsys commented Jan 26, 2022

I just tried building blocknative using RollUp 2.66.1 with the default settings and it did not make any difference.
I guess it's better to wait and see if the Vite team will have anything to say about it.
In the meanwhile I can do some workaround.

@theurgi
Copy link

theurgi commented Jan 26, 2022

@imsys I also came across this article by @FbN which addresses this problem by polyfilling esbuild. Here's the associated gist.

I feel like this is something that ultimately needs to be addressed by Vite and that Evan You's vitejs/vite#728 (comment) is totally tone deaf on this issue.

We're not even talking about top level dependencies here. It's highly likely that somewhere in your dependency tree you will have at least one sub-dependency that references the Node environment, especially when working with crypto libraries which (probably for security reasons) don't tend to update frequently.

The problem we're running into is: What is happening and where is it happening?—Is it a problem with SvelteKit? Vite? rollup? esbuild? Some combination of them?

It looks like Vue3 still simply uses rollup and does polyfill Node globals by default. I'm guessing this will change when they switch to Vite and a lot of dApps that use Vue3 and Web3Modal or Onboard will break if they update.

I think a reasonable solution is for Vite to create a simple configuration option to polyfill the Node env for projects that need it—and just have it work.

@imsys
Copy link
Author

imsys commented Jan 26, 2022

Funny thing is that Vite .ts code is transpiled to .cjs and run on NodeJs. xD

@taylorjdawson
Copy link
Contributor

@imsys This is a great discussion! But for house keeping reasons can this issue be closed?

@imsys
Copy link
Author

imsys commented Feb 7, 2022

@taylorjdawson, yes sure, it's more a problem in Vite side, so we can close this.

@imsys imsys closed this as completed Feb 7, 2022
@lnbc1QWFyb24
Copy link
Collaborator

@imsys I finally managed to get V2 of Onboard (with all wallet modules including hardware wallets) working in SvelteKit (vite) the other day and some of your comments above helped a lot, so thanks for that!

You should give V2 a try with Vite when you get a minute and let me know how you go. There is a build environments section that has the svelte.config that works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants