Discontinue callback support

[levino]

Stop supporting callbacks in the API. Just support promises, it is the future. If anyone is old-school and wants callbacks, they can use https://www.npmjs.com/package/callbackify

[nivida]

Thanks for opening this discussion @levino.

I totally agree that the support of callbacks should get removed. The current plan for the next week is to fix as many bugs as possible and to improve the documentation. Just to be sure the things are running as expected after the refactoring.

If the broader community agrees with this would I add a deperecation warning for the callbacks method in version 1.0.0-beta.41 or 1.0.0-beta.42 and will inform then when exactly the callbacks will be removed.

[leonprou]

But why don't the project uses a proper semver versioning style? So may breaking changes issues could be avoided. By removing callbacks a unexpected number of apps and projects would break..

[nivida]

I totally agree with you. A semver versioning would be great and would provide more transparency about which kind of change the new version is. But I will first improve the stability of this project before I call it a 1.0 stable version. I will not just remove the callback possibility from one release to another without informing the community some months before. The goal is to show a console warning on usage of a callback and to declare it as deprecated in a release announcement.

[sshelton76]

I wouldn't put this into 1.0
There are reasons other than being old school that people might be using call backs. Just cutting them out this close to release is going to make work for people when it isn't necessary. Maybe target this to 2.0?

[nivida]

@sshelton76 👌

We could remove them in a 1.x version later.

[sshelton76]

@nivida

We could remove them in a 1.x version later.

I'd like to recommend CB removal for 2.0, because 1.x would be too soon. This is a fundamental change to the way people have used the library in the past and it constitutes a major API change that would break A LOT of existing code. For 1.x we should go through all the callbacks and have it throw a deprecation warning though, first through console.debug, then console.warn and finally console.error at around 1.5.x.
I also agree we shouldn't add more of them.

[levino]

Changing the API of any method requires a release as a breaking version, so in this case a version 2.x. But there does not need to be any warning. Version 2.x should just come from the beginning without callback support. People who then update from 1.x to 2.x expect that the old code needs adjustments. They should RTFM before acting. If one is nice one can give them some tips on how to callbackify the promise returning functions with a quick example. That should be it.

[sshelton76]

I both agree and disagree.

Let me first state up front, no one hates callbacks more than I do. They are a messy kludge and the prevalence of them and the bugs they introduce is one reason I stopped using node on the server side and migrated most of my own projects to golang years ago.

However...

Going from callbacks to promises or async/await is not the same class of refactor as say going from a string to an object. You are no longer talking about a simple refactor, you're talking about a change to style and expectations, in short, it's a rewrite, not a refactor.

Having as much heads up as possible on that is critical if we want to encourage large scale serious uses of the library, and yes we do want that because it limits the issues from everyone trying to roll their own and the ensuing chaos from such endeavors.

In my case, I inherited an enterprise project with about 100 different for lack of a better word "oracles". All of which must function, all of the time.

Many of these oracles are still on 0.2x and using callbacks, lots and lots of callbacks.
There are 300k lines of code between them and the bloody callbacks are everywhere.

We could freeze on what we've got. But of course things like the ABIEncoder v2 as well as numerous security and bug fixes would be missed. Therefore I have to create a migration plan and get it approved and get a team ready to implement it. This takes time. Callbackifying might help, and would probably buy some time if it doesn't introduce new bugs, but it also breeds more of the same type of code I'm already dealing with.

Ergo, giving it some time with warnings gives me actionable intelligence I can use to prioritize our resources.

I assume that I am not an edge case here, but simply part of a segment with some special considerations. If we want largish orgs to adopt us, we do need to realize that there is some institutional inertia that is inherent in any business which will utilize the project.

Many companies don't have devops style "move fast and break things" mantras, but instead they need to know that things will work and continue to work at least on the 12 to 24 month horizon once they put forth the effort to build on top of this.

This is why I recommend, we always deprecate features removing them only when necessary, and give as much warning as possible about deprecation and the time remaining before deprecation is over and the feature removed.

Just my 2c.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions