0.20.11 no longer exposes Bluebird interfaces

[ivarconr]

Environment

Knex version: 0.2.11
Database + version: PostgreSQL 10
OS: linux

Bug

The newly released knex version (0.20.11) breaks my application.

TypeError: this.db.select(...).from(...).limit(...).whereRaw(...).orderBy(...).map is not a function

It works just fine by rolling back to knex version 0.20.10.

[ivarconr]

...This happens because you removed "Bluebird" dependency and returns a pure promise instead. This should have been a major change probably.

[briandamaged]

Howdy, and sorry about that!

We actually were debating quite a bit about whether or not this should be a major release. We ultimately decided to treat it as a minor release because the knex documentation did not officially claim to support the Bluebird interfaces. (ie: the interfaces were simply leaked due to the way the code was structured).

In either case, you can make the interfaces available again by wrapping the knex calls with Bluebird.resolve(..). For example:

// Split onto multiple lines for readability
Bluebird.resolve(
  this.db.select(...).from(...).limit(...).whereRaw(...).orderBy(...)
).map

[skreis]

That was surprising, indeed. Am I correct in reading the above that the README.md may be outdated? Particularly this bit:

  // .map over the results
  .map(function(row) {
    console.log(row);
  })

  // Finally, add a .catch handler for the promise chain
  .catch(function(e) {
    console.error(e);
  });

Thanks for a great library!

[ivarconr]

Yes that will work @briandamaged, but changing my code anyways it makes more sense to just adopt async/await style anyways.

But as @skreis points out your documentation kind of documents a bluebird like API.

Have you considered adopting a more "semver" like versioning? The current change is kind of dangerous as most people will use the "knex": "^0.20.0" syntax in their package.json and documented usages of the knex library breaks with the 0.20.11 release. Most npm user get it automatically because one expect patch releases to not change APIs.

Any reason you are not ready to move towards a "1.0.0" release?

Anyways thanks for all the great work with knex, it has severed me well for many years!

[elhigu]

Any reason you are not ready to move towards a "1.0.0" release?

@ivarconr it has been discussed already years ago #1331 I would be happy to make current knex to be released as 1.0 at any time.

[kibertoad]

@elhigu For me #2796 is one last missing feature before I personally would be happy with 1.0.0 release (and also we probably should drop Node 8 prior or together with that). Last time I asked Tim about what is holding 1.0.0 back at this point, he said that he's fine with us deciding whatever makes sense, so I guess we can do it when we agree among ourselves that it is time :).

[kibertoad]

@ivarconr We officially didn't support bluebird interface for a long time, though, it worked mostly by accident. See https://github.com/knex/knex/blob/master/UPGRADING.md

[kibertoad]

@skreis Please correct me if I'm wrong, but in the example that you've given wouldn't call to knex resolve into array that will be passed down the promise chain, and .map() would be invoked on array?

[elhigu]

@elhigu For me #2796 is one last missing feature before I personally would be happy with 1.0.0 release

I don't see any reason why that couldn't be included for example in 1.1.0 since it is not a breaking change. There are always missing features, which would be nice to have (like upsert is my favorite missing thing).

[kibertoad]

@elhigu So you would be in favour of making next major release (when we drop Node 8 and potentially change some of pooling logic that @briandamaged is cooking up now) the 1.0.0?

[skreis]

@skreis Please correct me if I'm wrong, but in the example that you've given wouldn't call to knex resolve into array that will be passed down the promise chain, and .map() would be invoked on array?

@kibertoad yes, I think that would be the case if it wasn't part of the knex.createSchema Promise chain; I may be doing something incorrectly, but when I run the example verbatim with 0.20.11, I see the following error message:

TypeError: knex.schema.createTable(...).createTable(...).then(...).then(...).then(...).then(...).map is not a function

[kibertoad]

@skreis You are right, we should fix that example.

[jstenmark]

If someone looking for a quickfix:

from:

exports.seed = async knex => {
  const data = await knex
    .select('id')
    .from('table')
    .map(({ id }) => id)
}

to:

exports.seed = async knex => {
  const data = (await knex
    .select('id')
    .from('table')).map(({ id }) => id)  
}

[kibertoad]

Yup, that is completely valid fix. Either that, or storing query result in array before calling map in it.

[ZuBB]

Howdy, and sorry about that!

We actually were debating quite a bit about whether or not this should be a major release. We ultimately decided to treat it as a minor release because the knex documentation did not officially claim to support the Bluebird interfaces. (ie: the interfaces were simply leaked due to the way the code was structured).

@briandamaged Its not my business but still you can do following

• revert commits that are made on top of 0.20.11
• revert commit that drop bluebird wrapping
• release 0.20.12 with bluebird wrapping with some notes why that has been done
• revoke 0.20.11 from npm
• revert of those from step1 and step2 and target these changes as 0.21

[briandamaged]

@ZuBB : Unfortunately, it's not so simple. The Bluebird refactoring was a very high-impact change across the entire codebase. There are several other enhancements / fixes in the 0.20.11 release that would need to be completely rewritten from scratch if the Bluebird-related changes were reverted.

I proposed a work-around in this other thread that you can try:

#1588 (comment)

Please let us know if this addresses the immediate problems that you are observing. If so, then we might be able to reformulate this approach into something that is better integrated with knex. Thx!

[ZuBB]

Please let us know if this addresses the immediate problems that you are observing. If so, then we might be able to reformulate this approach into something that is better integrated with knex. Thx!

I just tried to suggest a way how to make life easier for your users. I was not aware of internals. as for my case: we still far behind current versions (0.14 for RC and 0.17.6 for dev). so we will face it in couple months or so.

sorry that I did not explain that.

your team is doing great job. thanks a lot for an efforts

[briandamaged]

Ah -- no worries!

Yeah, we posted that potential work-around a little over a week ago, but unfortunately nobody has provided us with any feedback on it yet.

For anybody else who's following this thread, you can find the potential work-around here:

#1588 (comment)

If that addresses your issues, then please let us know. If it does, then we can look into creating a more formal solution around the concept. Thx!

[ungrim97]

Yeah. So this change just massively broke a bunch of my applications. Thanks for that.

I can understand the rational behind wanting to not use Bluebird. But this was absolutely a breaking change and should very much have been a major version change. Even if the interface wasn't documented, it is clear that people have come to use it.

[kibertoad]

@ungrim97 Sorry for that. We were removing bits and pieces of Bluebird promises over quite a long period of time, and complaints around that were really minor, so we really underestimated the impact that dropping Bluebird from transactions would have. In a hindsight yes, it should have been handled differently.

[ungrim97]

@kibertoad Nod. It is what it is. I am mostly salty right now cause I have just spent the day trying to work out why my application fell over when I hadn't made any real change here.

As I said I can understand why, though I personally feel that bluebird fixes some specific lacking features and implementation designs over native promise.

For me I was using the .map shorthand provided by bluebird as this has both concurrency support and saves doing .then(results => results.map((result) => { thingWithResult(result)}));

I suppose interoperability with Native Promises and async/await won out.

I would recommend doing something similar to sinon and adding a usingPromise method that allows the user to set a Promise implementation onto their promise chain.

Also I would suggest adding a big notice to the top of the Documentation as that would have saved me several hours of debugging

[kibertoad]

@ungrim97 Good point about documentation part, will do.

[elhigu]

Also in addition to the separate upgrade doc I would like to see the listing of breaking changes in the changelog as well. Those used to have a separate section in changelog.md when ever minor version updates were made.

[ungrim97]

@elhigu I agree. Though I suppose these weren't deemed as breaking changes originally as the interface wasn't documented as supporting the features being used.

This is one of those Bug becomes a Feature. Is fixing a bug a breaking change....Depends on who is relying on the bugs behaviour

[elhigu]

@ungrim97 it is a fine line where to draw and it must be decided separately for every bugfix... but this was caused of removing the bluebird and that was clear to every maintainer that it was a breaking change.

[kibertoad]

@elhigu Technically we were bumping major version for that particular reason for quite some time already, and just assumed that people had enough time to move on already by the fourth batch. If you check notes for 0.18.0, 0.19.0 and 0.20.0, all of them mentioned removing parts of Bluebird functionality, and UPGRADING.md already says * Knex returns native promises instead of bluebird ones now. You will need to update your code not to rely on bluebird-specific functionality; with regards to 0.18.0+. I assume neither of these versions broke code in a way that affected lots of people, which is why they didn't notice until next batch of changes landed within 0.20.x series.

[kibertoad]

But yeah, not everyone knows what Bluebird means in this context and were just confused that certain methods went away; and warning wasn't big and scary enough; and actually scary breaking changes came later. Yes.

[elhigu]

@kibertoad Yep, I know that the change happened during many releases and that all instructions how to cope with them were documented in UPGRADING.md.

But the changelog (which is also included in docs page) did not have any mentions about breaking changes. That has been usually the main document people are following when libraries get version updates (also the doc I'm browsing through when I update libs on my own projects). So I would like to see breaking changes also somehow emphasized in changelog.md even that the exact upgrade instructions are in the separate file.

[kibertoad]

@elhigu Yes, definitely, will add this today.