Fix: ensure using this.provider as the context this
#3721
Conversation
chendatony31
changed the title
thisthis
|
Is it really necessary for the context to be |
Yeah, We've got a problem, and now I have to detect the context whether is the More discussion may check here: #3649 (comment) |
|
This makes sense but will need to test it out later today and probably add a testcase for this |
|
Here's a codesandbox with a reproduction of this issue with |
There was a problem hiding this comment.
this looks good to me, binding provider context is relatively harmless in this situation and good practice for this style code. it's a little strange though because callbackifiy should preserve the this but maybe due to the changes in the pr mentioned that has change.
|
Any idea when this will get merged and released with tag 1.3.1 ? |
Update to resolve web3-core-requestmanager issue reported in web3/web3.js#3721 Assume a provider is passed to DydxClient, where the request function uses the 'this' keyword (for example to ensure initialization of the provider, perhaps fetching some data). When calling functions, such as: `client.eth.exchange.deposit(....)` The flow will eventually reach: 'RequestManager.send' In this, when calling the provider.request, we do not bind the provider, thus the value of 'this' is undefined, and any call to 'this' will result in an error due to undefined value. The fix will add the '.bind(this.provider)' which is currently missing, and make the 'this' value to be defined to the provider initially provided to the client.
Update to resolve web3-core-requestmanager issue reported in web3/web3.js#3721 Assume a provider is passed to DydxClient, where the request function uses the 'this' keyword (for example to ensure initialization of the provider, perhaps fetching some data). When calling functions, such as: `client.eth.exchange.deposit(....)` The flow will eventually reach: 'RequestManager.send' In this, when calling the provider.request, we do not bind the provider, thus the value of 'this' is undefined, and any call to 'this' will result in an error due to undefined value. The fix will add the '.bind(this.provider)' which is currently missing, and make the 'this' value to be defined to the provider initially provided to the client.
Description
The bug is brought in this PR: #3649
It may cause some problems when calling
provider.request. As the contextthismaybewindowsometimes, so we should make sure the context is theprovider.Type of change
Checklist:
npm run dtslintwith success and extended the tests and types if necessary.npm run test:unitwith success.npm run test:covand my test cases cover all the lines and branches of the added code.npm run buildand testeddist/web3.min.jsin a browser.CHANGELOG.mdfile in the root folder.