Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Avoid constant creation of new web3 object #1171

Merged
merged 7 commits into from
Aug 3, 2020

Conversation

dasanra
Copy link
Collaborator

@dasanra dasanra commented Jul 29, 2020

Currently we are creating a new web3 object each 3 seconds.

When we query for network info it's enough to check with current web3 instance. Web3 update logic should be handled elsewhere.

@github-actions
Copy link

github-actions bot commented Jul 29, 2020

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 8 0
Ignored 3 N/A
  • Result: ✅ success

  • Annotations: 8 total


[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods


Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Jul 29, 2020

Travis automatic deployment:
https://pr1171--safereact.review.gnosisdev.com/app

@@ -99,7 +99,6 @@ export const getProviderInfo = async (
web3Provider: string | Provider,
providerName = 'Wallet',
): Promise<ProviderProps> => {
web3 = new Web3(web3Provider)
const account = await getAccountFrom(web3)
Copy link
Member

@mmv08 mmv08 Jul 30, 2020

Choose a reason for hiding this comment

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

would it be possible to make it use passed web3 instead of using a global module variable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, if it's more explicit it's not a problem. Anyway we are always using a getter to get that web3 instance (from the same file)

@ghost
Copy link

ghost commented Jul 30, 2020

Travis automatic deployment:
https://pr1171--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Jul 30, 2020

Travis automatic deployment:
https://pr1171--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Jul 31, 2020

Travis automatic deployment:
https://pr1171--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Jul 31, 2020

Travis automatic deployment:
https://pr1171--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Aug 3, 2020

Travis automatic deployment:
https://pr1171--safereact.review.gnosisdev.com/app

@dasanra dasanra merged commit f374315 into development Aug 3, 2020
@dasanra dasanra deleted the fix/get-web3-memory-leak branch August 3, 2020 09:30
@mmv08
Copy link
Member

mmv08 commented Aug 3, 2020

Sorry for not saying this earlier: So it was implemented this way because of some weird behaviour in onboard.js when switching providers, I don't remember what exactly it was. The thing that if user changes a provider we need to use it to reinitialise the contracts and stuff. But now it looks like setWeb3 function from getWeb3.ts takes care of it and thus we don't need to recreate it.

@mmv08 mmv08 mentioned this pull request Aug 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants