coopermaruyama/react-web3

Breaking changes for using MetaMask

Closed this issue · 9 comments

Hello. Accordingly to this topic https://medium.com/metamask/https-medium-com-metamask-breaking-change-injecting-web3-7722797916a8
Soon some parts of this module will not to be working :( Can it be updated?

Nvm, done changes in my project to handle this case

Cool.

This library depends on web3 already being available on the window context, and I don't think it makes sense for it to choose a node/network for you.

Hi, I'm Dan from MetaMask! I think this issue should be re-opened, and I'm willing to maybe make the change myself.

I think this is a great component, I actually use it in a small web dapp I made.

OP is correct here, there is a breaking change that this module should account for. The best article for detailing the difference is here.

The short of it

Although web3 may be available, when accounts are not available, you should call let accounts = await ethereum.enable() to ask the user to sign in. This is both an improved user experience (no more telling the user to unlock metamask, just show the prompt), and improves privacy, because it means MetaMask is no longer showing accounts to sites that don't get explicit permission.

I actually came here to fork and maybe just add this change myself, so in a way I'm just commenting here first to make sure you'd approve the change. We made this protocol change in tandem with Mist, Coinbase, Cipher, imToken, and every other web3 browser that participates in the EIP process, so this should be considered the new preferred sign-in UX.

@danfinlay Thanks for your input! I would 100% approve that change if you were kind enough to add this. I will reopen the issue for this.

Sorry, I got held up on some basic things in your build system:

Error in ./test/provider-styles.test.js

  Module parse failed: /Users/danfinlay/Documents/Development/ethereum/node_modules/react-web3/test/provider-styles.test.js Unexpected token (14:6)
  You may need an appropriate loader to handle this file type.
  |   it('should load default stylesheet', () => {
  |           const wrapper = mount(
  |             <Web3Provider>

Maybe there's a dev dependency missing?

Was this when running npm test? My instinct tells me this is due to whatever bundler was involved (e.g. webpack) not recognizing JSX, most likely because the file is using .js extension rather than .jsx. In the webpack config that is included in this package, webpack is configured to parse JSX in both .js and .jsx files

Was this when running npm test?
Yes.

In the webpack config that is included in this package, webpack is configured to parse JSX in both .js and .jsx files
Just weird b/c I do the same thing in other projects w/o this problem. I'll try to revisit later.

My issue was that the file path included an underscore, and that apparently breaks babel-loader:
babel/babel-loader#173

Wow weird. Never would’ve guessed. Thanks