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

Use any-promise for everything #1415

Merged
merged 1 commit into from
Mar 13, 2018
Merged

Conversation

lionello
Copy link
Contributor

@lionello lionello commented Mar 2, 2018

any-promise was already in use in some submodules, except these two were using bluebird. This patch uses it uniformly.

@lionello lionello force-pushed the 1.0-promise-native branch from ab2683d to 301b82c Compare March 4, 2018 01:54
@coveralls
Copy link

coveralls commented Mar 4, 2018

Coverage Status

Coverage decreased (-0.08%) to 85.859% when pulling 301b82c on enumatech:1.0-promise-native into 2a4a722 on ethereum:1.0.

@onetom
Copy link

onetom commented Mar 4, 2018

One reason for this PR is that we noticed that sometimes promise rejections were not triggered.

As we were debugging we saw some DOM elements were created within promise management code by the bluebird library, instead of using the native Promise object available in the browsers (except IE 11 & Opera Mini.

The any-promise library also says:

NOTE: You probably want native promises now
...
If no preference is registered, defaults to the global Promise for newer Node.js versions. The browser version defaults to the window Promise, so polyfill or register as necessary.

We were concerned that such DOM trickery - which is not really necessary in 2018 - might interfere with web frontend frameworks (eg Vue.js), especially in their development mode with hot-code-reload.

So far we couldn't prove such interference because our issues seems stem from a combination of regressions and bugs in geth and web3.js, but it definitely helps the debugging experience if we don't have to wade through ancient DOM-magic-promise code in addition to the million other layers of the software.

@OnlyOneJMJQ
Copy link

+1 because this fixed #1386 for me.

@frozeman frozeman merged commit f9f3e55 into web3:1.0 Mar 13, 2018
@cgewecke cgewecke mentioned this pull request Oct 29, 2019
@cgewecke cgewecke mentioned this pull request Feb 21, 2020
13 tasks
nachomazzara pushed a commit to nachomazzara/web3.js that referenced this pull request Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants