feat: combine filters in .wait() method

[mi3lix9]

Currently, it's not possible to combine filters in one code. Instead, every filter has its own function .waitFor(), .waitFrom(), etc...

I suggest adding params to .wait() function with filters, like this:

await conversation.wait({
  for: ":text",
  from: ctx.from?.id
});

[KnorpelSenf]

This is a good suggestion to specify multiple conditions that all need to be true. How would you suggest that people can specify that one of several conditions should be true? For example, I'd like to wait either for a photo, or the /cancel command.

[mi3lix9]

This is a good suggestion to specify multiple conditions that all need to be true. How would you suggest that people can specify that one of several conditions should be true? For example, I'd like to wait either for a photo, or the /cancel command.

I think this may add more complexity so I suggest to filter using ctx or something like this:

ctx = await conversation.wait({
  for: [":photo", "/cancel"],
  from: ctx.from?.id
});

if (ctx.message.text === "/cancel") {
  return;
}

[KnorpelSenf]

ctx = await conversation.wait({
  for: [":photo", "/cancel"],
  from: ctx.from?.id
});

if (ctx.message.text === "/cancel") {
  return;
}

I'm afraid that this won't work well. It implies that the plugin should check if the strings start with / and then perform a wildly different logic than for everything else (which will be a filter query).

Most importantly, it is not a general way of combining the predicates. For example, we also have waitUntil which people can use to pass custom predicates. Your suggestion does not offer a way to work with those.

I agree that combining wait predicates is a desirable thing to do, and I'd love to have a good abstraction.

Until now, we can already use conversation.waitUntil(ctx => ctx.hasCommand("start") || ctx.hasQuery(":photo")). This can be combined in arbitrarily complex ways using AND and OR and what not. However, it requires people to abstract their own lambda, so it looks like it's not as clean as the good old middleware trees that poeple could use to combine middleware filtering (before we had conversations).

I do not consider this issue as blocking. In other words, if we find a great way of combining things, I'd be happy to implement it. However, if we don't, I'll release 1.0 anyway, but in that case I'm of course open to adding it for a future feature release if we happen to find the right abstraction after 1.0.

[mi3lix9]

I've just known about waitUltil(), I think if I knew it earlier I wouldn't open this issue, but after I see your examples I think we can let wait() for simple filters and waitUntil for complex ones

I suggest postponing this feature after 1.0 release, we may get better ideas :)

[KnorpelSenf]

@Mi3liX9 what do you think about this?

const textOrPhotoMessage = await Promise.race([
  conversation.waitFor(':text'),
  conversation.waitFor(':photo'),
])

This does not work yet, but it is feasible to implement it. It is the logical OR for any operations.

Analogously, it would be great to be able to do this:

const text = await Promise.all([
  conversation.waitFor(':text'),
  conversation.waitFrom(ctx.from?.id),
])

I am not sure if this is possible to implement, but I would not mind to try. It is the logical AND for any operations.

If we want, we could add aliases to make it easier to use, such as conversation.some and conversation.every or perhaps conversation.any and conversation.all.

[mi3lix9]

I like it, it is readable and easy to use, With alias will it be like this?

const textOrPhotoMessage = await conversation.any([
  conversation.waitFor(':text'),
  conversation.waitFor(':photo'),
])

const text = await conversation.all([
  conversation.waitFor(':text'),
  conversation.waitFrom(ctx.from?.id),
])

It's easy also to be nested too

const textOrPhotoFromUser = await conversation.all([
  conversation.waitFrom(ctx.from?.id),
  conversation.any([
    conversation.waitFor(':text'),
    conversation.waitFor(':photo'),
])

[KnorpelSenf]

Correct, that's exactly what I meant.

[mi3lix9]

It would be cool if we can use forms also I don't know if it's possible but maybe something like this:

const text = await conversation.all([
  conversation.form.text(),
  conversation.waitFrom(ctx.from?.id),
])

[KnorpelSenf]

Yes, this will automatically be possible if we implement the first suggestions. Forms are just syntactic sugar for wait calls with filters, so anything that works with wait calls will also work with forms without any changes.

[mi3lix9]

I tried this piece of code

const text = await Promise.all([
  conversation.waitFor(':text'),
  conversation.waitFrom(ctx.from?.id),
])

And it doesn't work

It behaves like we haven't added Promise.add at all

it needs two updates to work

[KnorpelSenf]

Exactly. That's what I meant when I said:

This does not work yet, but it is feasible to implement it.

:)

[mi3lix9]

I compared my idea with yours, and still see your way is great and easy to read. I like conversation.any([]) & conversation.all([])

[WcaleNieWolny]

for anyone interested: I wrote a very simple work around to combine the following

conversation.waitFrom() and await conversation.waitFor('message:text')

function waitFromMsg(
  ctx: Conversation<SessionContext>,
  user: number,
): Promise<Filter<SessionContext, 'message:text'> & { from: User }> {
  const predicate = (ctx: SessionContext): ctx is Filter<SessionContext, 'message:text'> & { from: User } =>
    ctx.from?.id === user && grammyContext.has.filterQuery('message:text')(ctx)
  return ctx.waitUntil(predicate)
}

[KnorpelSenf]

Context.has.filterQuery("message:text")(ctx) can be simplified to ctx.has("message:text"). Alternatively, you can pull it out into a variable hasText and only call hasText(ctx) which saves a few CPU cycles.

[KnorpelSenf]

With the unreleased version 2.0, this suggestion is already possible.

const textOrPhotoMessage = await Promise.race([
  conversation.waitFor(':text'),
  conversation.waitFor(':photo'),
])

However, if you send a photo, the waitFor(":text") call will be left hanging. As soon as the next text message arrives, it will resolve. Due to the way race works, it will have no effect, thus effectively capturing and suppressing the next text message. That is undesired.

We can consider making wait calls cancellable via a cancellable promise. This would enable us to implement conversation.any which takes care of cancelling all other wait calls as soon as one of them resolves.

With v2, every update is only returned from a single wait call. There is no parallel resolving of wait calls. This means that the following suggestion becomes impossible (or rather, it means something else now).

const text = await Promise.all([
  conversation.waitFor(':text'),
  conversation.waitFrom(ctx.from?.id),
])

Instead, we can consider returning a promise with extra methods that lets people narrow down the update further. Something based on chaining like conversation.waitFor(":text").andFor(":forward_origin") could be used to wait for forwarded text messages. However, I am not in favour of adding more methods to promises, so I would ideally like to see a different proposal. Adding a .cancel to the promises is already something I'm not super comfortable with.