Ioredis: dropping node < 4

[AVVS]

@luin what do you think? Can this make it into v3?
I'm primarily concerned about dependencies and being able to use some of the recent es6 features

[luin]

I definitely agree that we can drop the support for node < 4 so we don't need to worry about the BC for the older version, though I'm a little worried about the performance of es2015 with V8 (http://v8project.blogspot.kr/2016/12/v8-release-56.html?m=1).

[abritinthebay]

This would also open up dropping the pointless (given what it is used for here) Bluebird library and using Native Promises. As far as I can tell Bluebird is only used for functionality that Native Promises provides out of the box.

[AVVS]

bluebird is a) faster than native promises and b) has useful utils, so when you say pointless, thats highly debatable

On Feb 9, 2017, at 12:04 PM, Gregory Wild-Smith [e-mail] wrote:

This would also open up dropping the pointless (given what it is used for here) Bluebird library and using Native Promises. As far as I can tell Bluebird is only used for functionality that Native Promises provides out of the box.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/luin/ioredis/issues/408#issuecomment-278756444, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0Qq98TdLwqYZhLDB8yg7LBVtLXUmks5ra3FJgaJpZM4LCu7A.

[abritinthebay]

The only parts it is faster on is because it doesn't follow the spec. Neither do those utils. Bluebird is NOT promises. It doesn't follow the spec. It mostly follows the spec.

Following broken psuedo-implementations is an anti-pattern. Best to nip it in the bud as early as possible.

[AVVS]

I hear you, but these are not arguments, this is a personal opinion. Explain your position, tell what broken, whats working not as expected?
As far as I’m personally aware BB follows A+ spec and adds extra methods on top, which makes promises actually useful and pleasant to work with.

For example, basic promises are limited to .then, .catch and .all, but it leaves out all the actual usage patterns behind. Before promises control flow libs were highly popular due to hardly avoidable callback hell,
now they have their niche, but so do promises (when you can actually use them for control flow, like with BB). Of course, there is async/await, but its not released yet either, and it uses try/catch semantics, and that is debatable, too

So once again, please facts & arguments, not tunnel-visioned statements, such as BB is bad and native is good

On Feb 9, 2017, at 3:50 PM, Gregory Wild-Smith [e-mail] wrote:

The only parts it is faster on is because it doesn't follow the spec. Neither do those utils. Bluebird is NOT promises. It doesn't follow the spec. It mostly follows the spec.

Following broken psuedo-implementations is an anti-pattern. Best to nip it in the bud as early as possible.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/luin/ioredis/issues/408#issuecomment-278813909, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0e4d0aMOF0FjBntIU2xGzUZcdFczks5ra6YxgaJpZM4LCu7A.

[abritinthebay]

it leaves out all the actual usage patterns behind

Great. Add those. Don't call it Promises, because it's not. Don't reimplement a (not-quite up to spec) version of Promises to add it.

As far as I’m personally aware BB follows A+ spec and adds extra methods on top

Yeah, it doesn't. For example new Promise is an anti-pattern in Bluebird. Why? Because speed-wise it drops back down to native levels. Their benchmarks cheat a lot in this way. They also don't allocate arrays for promises' handlers. Meaning Promise branching is limited to incomplete.

Basically it's a bunch of things to win micro-benchmarks while only adhering to as much of the spec as they can do be "compliant".

However, it should really be about why does ioredis even need Bluebird?

Answer: it doesn't unless it's for compatibility for old versions of Node.

Evidence: here is the entire list of Promise-related APIs ioredis uses:

new Promise
Promise.reject
Promise.resolve
Promise.then

That's it. 100% Native compatible without using any extra library. Also 100% as fast as Bluebird because the raw reject/resolve/then calls are the same speed and Bluebird performs approx the same when used with new (instead of their promisify implementation).

So, long story short: why keep it? It adds nothing but bloat.

[AVVS]

Still nothing justifies that. In my view every argument here is just kudos to the clever implementation of BB promises.

On Feb 9, 2017, at 4:08 PM, Gregory Wild-Smith [e-mail] wrote:

it leaves out all the actual usage patterns behind

Great. Add those. Don't call it Promises, because it's not. Don't reimplement a (not-quite up to spec) version of Promises to add it.

As far as I’m personally aware BB follows A+ spec and adds extra methods on top

Yeah, it doesn't. For example new Promise is an anti-pattern in Bluebird. Why? Because speed-wise it drops back down to native levels. Their benchmarks cheat a lot in this way. They also don't allocate arrays for promises' handlers. Meaning Promise branching is limited to incomplete.

Yes, it’s slow. And it’s slow because this effectively allocated a new function, and a new scope on each invocation. That’s why it’s done that way. It’s an anti-pattern without any regard to bluebird, if
new Promise(fn [, ctx]) supported an optional context, then it would’ve been possible to make this as fast or event faster. But it doesn’t - hence the anti-pattern
Basically it's a bunch of things to win micro-benchmarks while only adhering to as much of the spec as they can do be "compliant".

However, it should really be about why does ioredis even need Bluebird?

Answer: it doesn't unless it's for compatibility for old versions of Node.

This is one thing I agree - it doesn’t need it on node 4+, but a) it would be a huge breaking change b) literally any developer I know would bring BB or do a global Promise remap to bring extra functionality and save dev time
Not to mention here is the entire list of Promise-related APIs ioredis uses:

new Promise
Promise.reject
Promise.resolve
Promise.then
That's it. 100% Native compatible without using any extra library. Also 100% as fast as Bluebird because the raw reject/resolve/then calls are the same speed and Bluebird performs approx the same when used with new (instead of their promisify implementation).

So, long story short: why keep it? It adds nothing but bloat.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/luin/ioredis/issues/408#issuecomment-278817877, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0dGQr7NFEH7ao4VHo4FEPnh-GfNZks5ra6ptgaJpZM4LCu7A.

[abritinthebay]

a) it would be a huge breaking change

As I demonstrated: this is literally not true. ioredis uses nothing of Bluebird. No breaking changes whatsoever.

literally any developer I know would bring BB or do a global Promise remap to bring extra functionality and save dev time

Ah, annecdata. The last refuge of the lack of argument. I can counter with - no developer I know would willingly add a bloated library when it wasn't required. Which it isn't for 99% of promise use.

[AVVS]

Its a huge breaking change, because it will ultimately break all the code that relies on BB functionality.

Imagine snippet of

redis.pipeline().get(x).hmget(y, one, two, three).exec().then(handlePipeline).spread((valueX, ArrayofValuesFromY) => {
    // stuff to do
})

This won’t work any longer.
Anything with tap won’t work - literally any code that was written that relied on BB utils won’t work.

On Feb 10, 2017, at 10:59 AM, Gregory Wild-Smith [e-mail] wrote:

a) it would be a huge breaking change

As I demonstrated: this is literally not true. ioredis uses nothing of Bluebird. No breaking changes whatsoever.

literally any developer I know would bring BB or do a global Promise remap to bring extra functionality and save dev time

Ah, annecdata. The last refuge of the lack of argument. I can counter with - no developer I know would willingly add a bloated library when it wasn't required. Which it isn't for 99% of promise use.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/luin/ioredis/issues/408#issuecomment-279034258, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0fnSzb7vx0NftTHnYKlquZPLLXSNks5rbLOOgaJpZM4LCu7A.

[abritinthebay]

So Bluebird isn't compatible with Promises. Great, glad we got that cleared up.

But fine, sure, breaking change. So is killing node < 4. Perfect time to do it then.

[AVVS]

it is a superset of Promises and is fully compatible, don’t mix these two terms ;)
Back to square one - if they are completely replaced it will break code and will reduce DAU one way or another, there would appear a fork for people who want it the other way.

I can see a solution that can work both ways - use either bluebird or native promise with bluebird prevailing if it’s available. That you can control - it won’t be a hard dependency, but will still be available for those who prefer it

On Feb 10, 2017, at 1:23 PM, Gregory Wild-Smith [e-mail] wrote:

So Bluebird isn't compatible with Promises. Great, glad we got that cleared up.

But fine, sure, breaking change. So is killing node < 4. Perfect time to do it then.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/luin/ioredis/issues/408#issuecomment-279069448, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0TXn6MpDrFaOD-NT0Pvh5m9Zpxkyks5rbNU_gaJpZM4LCu7A.

[abritinthebay]

That would be cleaner. Making it an optional peer dependency would be much less bloat-y.

[AVVS]

I’m pretty sure that won’t work either for some people npm might return warns/errors for this. Likely it shouldnt be listed as dep at all, but there should be a wrapper that includes it if it’s available. Somewhat similar to how hiredis was used earlier

On Feb 10, 2017, at 3:17 PM, Gregory Wild-Smith [e-mail] wrote:

That would be cleaner. Making it an optional peer dependency would be much less bloat-y.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/luin/ioredis/issues/408#issuecomment-279092811, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0YxI2njpAYjm2wMG1yDHW1bgcQ8aks5rbPAEgaJpZM4LCu7A.

[abritinthebay]

Hmm, good point, though it's just a warning - should not throw errors in npm >= 3. In effect _all_ peer deps are optional in npm.

[AVVS]

https://www.npmjs.com/package/any-promise - this could work for all cases

[abritinthebay]

That's a nice solution. I think @AVVS would still have the objection that this is a _potentially_ breaking change for people relying on Bluebird behavior further down the chain (even if it's a bad practice).

Which is fair - but that is an excellent solution to solving the compatibility layer.

[Salakar]

Bluebird is NOT promises. It doesn't follow the spec.

@abritinthebay pretty sure @petkaantonov and the guys over at https://github.com/promises-aplus/promises-spec would disagree with you, but... hey what do they know. 👍

Not entirely sure why you're against it, it keeps Promises consistent across versions - there has also been changes between node 4 - 7 in the native node promises implementation, such as better stack traces in the newer versions that have not been back ported to older versions.

no developer I know would willingly add a bloated library

Your concerns about 'bloat' are far fetched, this is server side code:

• If it was bundled for web usage then sure - I'd understand maybe.
• ioredis has <10 dependencies of which most of them have 0 dependencies themselves (bluebird included) this point is moot.
  
  
  
  • If your opinion of bloated is 1 module with zero dependencies then you're going to have a bad time in the npm/node ecosystem - better not use browserify then anymore in your projects, it has 119 dependencies too many for you. 🐱
  
  

That's it. 100% Native compatible without using any extra library. Also 100% as fast as Bluebird because the raw reject/resolve/then calls are the same speed and Bluebird performs approx the same when used with new (instead of their promisify implementation).

Would love to see your benchmarks on this, while you're at it can you benchmark memory usage.

Back on subject:
As for node support, I'm all for making node 4 the minimum version.

[abritinthebay]

it keeps Promises consistent across version

By having an interface that exposes incompatible APIs to Native promises and doesn't actually function according to the same rules? Ooookay.

Your concerns about 'bloat' are far fetched, this is server side code

MFW someone thinks server-side code can't be bloated 🙄 If you think that... well, that's horrifying.

[Salakar]

@abritinthebay never said it can't be bloated, server side code bloat is not an issue here on this lib, I'd say educate us with your experience on open-source libs and node but I feel we'd have better conversation talking to a brick wall. 👍

Back to topic.

@AVVS definitely should support 4+ in the next major release, had this discussion a while back with @luin - both agreed it was for the best and gives more flexibility.

Though having said that, my thoughts are start at version 6, its now in LTS - allows us to then use things like Proxy and gain a ton of performance benefits, e.g. a redis client I wrote that made use of Proxy and various other ES6 features (and god forbid... bluebird):

[abritinthebay]

I feel we'd have better conversation talking to a brick wall.

Funny, that's the impression I'm getting from you too. 🙄

Anyhow, _in more productive conversation_ I agree with you on the suggestion of Node 6 LTS _in principle_ but think you might be being a bit premature.

Lots of Node 4 LTS apps are still out there and it in fact doesn't even go into maintenance mode until April of this year - and it will still be supported until April of next year.

Given that I'd say it's premature to drop v4 but the point you make about extra features is a _great_ one for a future point on the roadmap.

[Salakar]

Funny, that's the impression I'm getting from you too.

Touché. 😛

Guess it depends when we'd want to release by, I'd suggest as soon as 4 is in maintenance unofficially drop it, it's not like anything drastic is going to change during the maintenance period really that'd break older versions? But, I can also see the need to keep it in support still until April next year. Ah, decisions.

I did originally write the cluster slot calculator and the double ended queue implementation used here to only support node 4+ but eventually just made it backwards compat to 0.12 just to get it in for release - so I guess it'd save hassle like that and thats the main thing really for me.

The only major thing that'd be good to utilise that I can see missing from 4 is Proxy (but it is there behind a harmony flag), const, let, classes etc are all there.

[abritinthebay]

I'd lean to _initial_ support being 4+ for the first upgrade. Using new fancy features for 6+ can then be a SemVer Major update and drop 4 compatibility (iterative updates and development is good, right?)

That way 4 gets first class support of _its_ features, it's a smaller update, it's faster to get to release, etc etc

Best of both worlds. As you said - the main difference for 6 would be Proxy and Proxy is really a whole thing by itself. No need to complicate matters.

My general rule is always go for the simplest solution and iterate complexity when needed.

[abritinthebay]

Incidentally - related to both Promises and Node v6 - Mikael Rogers had this to say on the perf differences:

"0.02ms additional overhead is that native promises clock have over bluebird on Node.js v6."

• https://twitter.com/mikeal/status/834516413536628737

So... given who that is that would be my cite for that figure (for what it is worth, that's basically what I've seen personally too). It's functionally identical.

Not to derail the conv again, but you _did_ ask. 😄

[AVVS]

@Salakar can I take a look at the experimental redis client with proxy? Wonder which techniques allowed for such a drastic speedup

[elmigranto]

Every side makes valid points. From what I've seen, most libs that find themselves in similar situation provide an option for user to set custom Promise constructor while defaulting to system one.

const redis = new Redis({
  promiseConstructor: require('bluebird') // if omitted, `global.Promise` is used
});

My personal opinion is that while Bluebird is definetly a very useful thing, it's least surprising to use global.Promise as a default. Imagine same argument about Date or Array, and I think it's quite obvious.

[abritinthebay]

Reasonably put. I think it's the exact opposite of surprising as a default though - it's a native type. The default being a native implementation should _never_ be surprising.

The default being a library should be surprising. Having a config option to override though is entirely reasonable.

[abritinthebay]

Doh, brain fail. Read it as "at least surprising". Never mind!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[AVVS]

@luin how are we on that? Node < 4 is deprecated and with new V8 we should be moving forward imo

[connor4312]

Node 4 isn't deprecated, it's in LTS maintenance for another six months. That's a big difference! After April it'll be EOL. While I understand the desire to use fancier and faster ES6 features--I am looking forward to being able to async/await all my libraries--not all consumers have the privilege of being able to upgrade quickly. The three year release-to-EOL cycle Node has is already on the aggressive side as far as LTS (Ubuntu LTS, for instance, have a five year cycle).

[AVVS]

IMO, it just stagnates growth, there were no major updates/fixes/features for many months now and the ability to drop node 4 is still something to consider. Of course, this would be a breaking change, but there is no reason to force any1 to upgrade at this point - if you cant use new version, just stick to current release, if you can - use the new one. Its actually common to only support latest LTS and beyond at this point to simplify transpiling, etc

[abritinthebay]

Agree on that. It wouldn't break Node4 it would just say "ok, we're at a new LTS release so we're going to focus on that moving forward".

Seems like a sane and absolutely reasonable decision from a management of resources point of view.

[naholyr]

Just a comment about dropping bluebird:

About performance: did you bench ioredis with/without bluebird, or just rely on provided micro-benchmarks? I think only the first is valuable, and should be tested.

Dropping bluebird should be considered, just replacing bluebird with i-promise for example would provide a configurable way for everyone, at no cost. I'm currently fighting with Bluebird handling "run-away" promises (that's a nice feature, but unwanted in this case), and I wonder how it will work when Node finally makes process crash on unhandled rejection, as this will only concern native promise… so you may suddenly have some promises that don't crash, and some that do, and that will be a surprise… if every bluebird-based module could at least provide a way to stick to native, it would be perfect for such cases.

[AVVS]

one way is probably ask for a promise implementation in the constructor and fallback to default available (ie native Promise)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[kdekooter]

We should definitely have native promises!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[kdekooter]

@luin would it be possible for you / the committers to make a decision about this issue, and not let it be wiped out by the _stale bot_?

It is 2018 now. Promise has been a first class nodejs citizen for quite some time now. ioredis should support this so one can have a native-promise-only codebase.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[kdekooter]

Is this project still alive? Or is the _stale bot_ running the show?

[abritinthebay]

Given Node < 4 has been End Of Life'd since 2016-12-31 and Node 4 itself is going to be EoL by 2018-04-30 plus the last serious update to the codebase was 7 months ago and there has been no interest in moving the codebase forward: in fact hostility against it....

I mean... I've moved on from this project assuming it's basically dead from a practical standpoint.

[naholyr]

What's the best alternative known other than util.promisifying node-redis?

Le mar. 20 mars 2018 à 19:38, Gregory Wild-Smith [e-mail]
a écrit :

Given Node < 4 has been End Of Life'd since 2016-12-31 and Node 4 itself
is going to be EoL by 2018-04-30 plus the last serious update to the
codebase was 7 months ago and there has been no interest in moving the
codebase forward: in fact hostility against it....

I mean... I've moved on from this project assuming it's basically dead
from a practical standpoint.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/luin/ioredis/issues/408#issuecomment-374711027, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AANEMxYxgTAq57l8AbJLLwpAZItESsRLks5tgUy3gaJpZM4LCu7A
.

[shaharmor]

It's not dead at all. If you have any contributions to make we will be glad to review them.

Regarding dropping node < 4, I agree that it's a good move and we should definitely move ahead in that direction.

@luin, let's talk and see how we can move this forward.

[jcstanaway]

I certainly hope it's not dead. It's the only library that supports Redis
Clusters. node-redis doesn't support that yet.

Though it would be nice if the various questions and issues that have been
raised in the past 2-3 months can be addressed. I wish I could submit PRs
to address the issues I've run into, but I simply don't have the time to
delve into a library I have no familiarity with. if it's not going to be
maintained, then it would be good to let the community know. I understand
that there was a plan to merge ioredis & node-redis, but that doesn't
appear to have had any activity either.

On Tue, Mar 20, 2018 at 4:54 PM, Shahar Mor [e-mail]
wrote:

It's not dead at all. If you have any contributions to make we will be
glad to review them.

Regarding dropping node < 4, I agree that it's a good move and we should
definitely move ahead in that direction.

@luin
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_luin&d=DwMFaQ&c=q3cDpHe1hF8lXU5EFjNM_A&r=N1Msa0ZexCqrdtWLgqlbMZlNrFKF4YmyP5otZCNJ7mmERCt9HPx5t646YC5_jvy8&m=wCT6Ly5D5OLHkl4Sb1cpdZlfijci_2OKI5Lul2KYJsk&s=kbUmzdvNIswTrwwbgWQrNvRvfmjd9UCpgYj2w2e_lBE&e=,
let's talk and see how we can move this forward.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_luin_ioredis_issues_408-23issuecomment-2D374770404&d=DwMFaQ&c=q3cDpHe1hF8lXU5EFjNM_A&r=N1Msa0ZexCqrdtWLgqlbMZlNrFKF4YmyP5otZCNJ7mmERCt9HPx5t646YC5_jvy8&m=wCT6Ly5D5OLHkl4Sb1cpdZlfijci_2OKI5Lul2KYJsk&s=Um1AasMuObdw-9f6B__vs7CpRpk2Nh3mWPCja44BidU&e=,
or mute the thread
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AVxAa79EvY7dZoBG7Enu5HvByCSKr5bxks5tgXqdgaJpZM4LCu7A&d=DwMFaQ&c=q3cDpHe1hF8lXU5EFjNM_A&r=N1Msa0ZexCqrdtWLgqlbMZlNrFKF4YmyP5otZCNJ7mmERCt9HPx5t646YC5_jvy8&m=wCT6Ly5D5OLHkl4Sb1cpdZlfijci_2OKI5Lul2KYJsk&s=zOqV5-5yA29mYatuFxiljOsXFqpd1LrJIyHVmrU4TzA&e=
.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[kdekooter]

Hi fresh bot again here. We need this!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[kdekooter]

Good day folks, fresh bot again. Still interested in a fix!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[kdekooter]

Good day folks, fresh bot again. Still interested in a fix!

[luin]

Just released ioredis v4.0.0-0. It's a major beta release that drop the support for Node.js < 6. We uses the native Promise by default instead of bluebird (and users can switch back via Redis.Promise = require('bluebird')).

The total package size dropping to 758KB from 1.4MB 😆.