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

Add Authereum provider #82

Open
wants to merge 1 commit into
base: master
from

Conversation

@miguelmota
Copy link

commented Sep 16, 2019

Add Authereum web3 provider

@miguelmota miguelmota force-pushed the authereum:authereum branch from d749806 to 16ae714 Sep 16, 2019
src/providers/index.ts Outdated Show resolved Hide resolved
@pedrouid

This comment has been minimized.

Copy link
Member

commented Sep 16, 2019

Looking good, something wrong with the build, going to check it out

@miguelmota miguelmota force-pushed the authereum:authereum branch 2 times, most recently from 421a2b1 to c72cb1b Sep 16, 2019
@miguelmota

This comment has been minimized.

Copy link
Author

commented Sep 17, 2019

Regarding the build, looks like it's running out of memory. You can use GENERATE_SOURCEMAP=false for the build since generating sourcemaps is memory intensive. More info here

@pedrouid

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

BTW @miguelmota I just tested the example locally with Authereum and I got the following error:

Uncaught DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.
@pedrouid

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

Also I tested the production build for the example locally too and disabling GENERATE_SOURCEMAP worked but I honestly never had to do this for any app I've built before and this is a very small one.

I also noticed that the app bundle size increased considerably with the authereum dependency from 799.35kb (master branch) to 1.19mb (authereum branch).

You should look into your dependencies and most importantly your webpack config because otherwise you will have to ask all apps to disable GENERATE_SOURCEMAP to integrate Authereum

@miguelmota

This comment has been minimized.

Copy link
Author

commented Sep 17, 2019

@pedrouid yeah you're right it's absurdly large. We'll work on removing dependencies to get it to a normal size. Agree that disabling sourcemaps shouldn't be required because of this. Thanks for trying it out and providing suggestions and I'll get back to you when it's improved

@miguelmota miguelmota force-pushed the authereum:authereum branch 2 times, most recently from 86d2e31 to 32ea7b5 Sep 17, 2019
@miguelmota

This comment has been minimized.

Copy link
Author

commented Sep 17, 2019

Deployment looks good now with removed dependencies

@pedrouid

This comment has been minimized.

Copy link
Member

commented Sep 18, 2019

Nice 👍 However I just tested again and I still get the missing localStorage issue running locally in development.

@miguelmota

This comment has been minimized.

Copy link
Author

commented Sep 18, 2019

I'll test it out; what browser were you using? and were you using incognito mode?

@miguelmota miguelmota force-pushed the authereum:authereum branch from 32ea7b5 to 6ff8abf Sep 24, 2019
@miguelmota

This comment has been minimized.

Copy link
Author

commented Sep 24, 2019

I wonder if you were using an old version because the latest version doesn't use localStorage at all. You should try pulling the latest changes and wiping node_modules before trying it again

@pedrouid

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

Hi @miguelmota, I'm happy to merge this PR but I keep getting this issue. I cloned again from authereum/web3connect repo and I got the same with localstorage. The version installed was 0.0.4-beta.8.

Screenshot 2019-09-27 14 44 53

Can you try it yourself on your machine? Fresh clone to try to replicate it.

@miguelmota

This comment has been minimized.

Copy link
Author

commented Sep 27, 2019

@pedrouid ah I see now, it must be because your browser has cookies and localStorage disabled in the settings. I'll try to figure out a way to work around that.

@miguelmota

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

Hey @pedrouid I'm curious to how you're able to interact with other providers that rely on localStorage (ie like Portis) with the recommended browser setting turned off

nausettings

We aren't able to maintain sessions or store the ephemeral keys if there's no way to store them in the browser by blocking all site data. The best we can do is to alert the user to enable site data to access the provider (like portis does)

@miguelmota miguelmota force-pushed the authereum:authereum branch from 6ff8abf to 683a96c Oct 4, 2019
@miguelmota

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

@pedrouid the authereum provider should no longer crash like that and it displays an error toastr if persistent storage is not available which is required. Many other providers won't also work with 3rd party storage disabled which is default in brave

Related ethereum/web3.js#3031

@miguelmota miguelmota force-pushed the authereum:authereum branch from 5234722 to c7a7698 Oct 12, 2019
@miguelmota

This comment has been minimized.

Copy link
Author

commented Oct 12, 2019

Hey @pedrouid is there anything we can do to help get this merged? The error you were seeing no longer exists (tested on brave with shield enabled). Would really like to start promoting web3connect to dapps but first need it merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.