knex: Cannot access the query object returned from an async function

I assume this is due to lack of understanding of the internals of Knex, but it is very common to have a async/await function that awaits on some other async call and then proceed to generate a Knex query which I would like to return without executing so I can continue to augment it.

Here is a very naive example that is currently impossible:

async function async getPostsForUser(userId) {
	const user = await this.getUser(userId);
  	return db.select('users').where('userId', user.id);
}

const q = (await getPostsForUser()).limit(10); // would expect Knex query

const result = await q;

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 1
  • Comments: 16 (8 by maintainers)

Most upvoted comments

async function async getPostsForUser(userId) {
	const user = await this.getUser(userId);
  	return { builder: db.select('users').where('userId', user.id) };
}

const q = (await getPostsForUser()).builder.limit(10);

const result = await q;

I expected promises to be chained and resolved one by one by every single await. Thanks for the example, maybe it should be included somewhere as a FAQ.

Having builder() or query() and execute() function sounds reasonable and in my opinion it could live in parallel with the current design that returns thenable.

If this comes up frequently, it will be a good idea. I wouldn’t add it there yet though, since that is common knowledge, which developers just need to know when they are working with promises.

Since querybuilder is a ‘thenable’ calling await on it will issue the query and return the result. The example is a bit confusing to me. The function takes userId but also runs a lookup from this userId -> user.id. Seems redundant?

There are other ways of solving this but it does not involve using await at all, which I’m guessing is what you really want.

It could live in parallel, but I’m strongly against it, because of various reasons (mostly that it will cause more harm than good for knex project which is already too bloated with various APIs it offers):

  • it would be more confusing to have multiple types of query builders going around, which some are thenables and others are not
  • that additional API is not much better than, just wrapping normal builder
  • it will cause more maintenance burden, when all knex parts need to make sure to work with non-thenable query builder,
  • there will be more related bug reports when there are confusion about it or when people find all corner cases where those query builders are not working properly
  • tl;dr this kind of seemingly small features which are easy to workaround outside of knex (doesnt add much value to the library) very easily add up to become big burden in the long run

If knex will have proper support for extending it with plugins, that would be good place to add this kind of less commonly needed APIs as separate packages. For now I suggest you just add your own wrapper method for passing query builder instance without executing them from async functions. Like:

function partialQuery(builder ) {
  return { query: builder };
}

otherMethod()
.then(values => partialQuery(knex.select(table).where('somefield', values.x)))
.then(knexBuilder => knexBuilder.query.orderBy('otherfield', 'desc'))
.then(results => queryResultsHere);

I don’t think I have anything else to add to this issue, those provided workarounds should be enough for anyone.

I just want a way to force it to not be automatically promisified.

By promise spec, that would require removing or disabling .then method from query builder. That would mean that after returning that query builder from async method one would have to somehow enable, .then method again… so your code would start to look something like this.

async foo() {
  return knex('foo').disableThen();
}

query = await foo();
let allTheFoos = await query.enableThen();

Unless disable method would be automatically set to re-enable then after it has been resolved for the first time. In that case if there would be some nested hierarchy where it is passed through multiple async funcs, you would need to disable it on every step… which just sounds too magical and hacky and would cause even more confusion. The way to get around this problem by wrapping the value is more clear solution to ever JS developer, whom are not familiar with all peculiarities of knex.

It has nothing to do with experience levels of JavaScript.

I believe that deep understanding how promises work has exactly something to do with that 🤔

@ivosabev that wouldn’t work. Returned query would not be thenable anymore and for example this wouldn’t do anything:

async function getUsers() {
  const organization = await getViewerOrganization();
  const query = knex
    .select('*')
    .from('users')
    .where('organization_id', organization.id)
    .query();

  return query;
}

query = await getUsers();
let users = await query;

Not knex problem. As @wubzz mentioned, that is just the way how promises / async / await works with thenables. If you want to return thenable from async function without causing it to resolve, you have to wrap it to for example inside an object for returning.