grammyjs/conversations

Rename `wait()` to `yield()`

Opened this issue · 2 comments

qcho commented

Hi,

I'm new to grammyjs and the conversations plugin. Great work btw! Just want to share my experience of reading the documentation.

My personal reading the documentation experience

On the first approach with the plugin I was really concerned about how would this plugin handle concurrent conversations at scale when reading examples such as:

async function greeting(conversation, ctx) {
  await ctx.reply("Hi there! What is your name?");
  const { message } = await conversation.wait();
  await ctx.reply(`Welcome to the chat, ${message.text}!`);
}

Once i reached the middle of the documentation i found Three Golden Rules of Conversations and I could infer that something really great was implemented under the hood.

Then almost at the end of the documentation you get the How It Works that greatly explains and covered my concerns.

Great documentation!

Proposal

I think the plugin would benefit with a minor naming change that will enable us (new users) to don't infer the "naive promise approach" until the very end of the documentation.

  1. Rename wait for yield method.
  2. Add a new wait method marked as deprecated for bg compat.
  3. Replace wait for yield on documentation examples.

Why?

  • yield is more expressive of what is actually happening.
    • I can infer the control of the process is being handled as explained in the how-to doc.
    • Even the documentation makes use of this keyword when explaining how it works "The wait method always yields a new context object representing the received update."
    • Isn't odd to have await ... wait() call? await conversation.wait(); vs await conversation.yield();

Obviously this is a minor constructive feedback on a great designed solution, i don't expect anything more than getting the opinion of the maintainers on this adoption topic.

I might as well air my thoughts on this. Personally, I believe .wait() is more appropriate. For a user who's not very concerned about the inner workings or, as you put it, "what is actually happening", wait is more intuitive. What is the bot doing? It's waiting for a new update. Moreover, .wait() is tied to so many other methods which apply a filter e.g. .waitForMessage(), .waitForCommand(), etc. These methods are aptly named such that one can easily tie them to the .wait() command. Can't imagine what they would be if .wait() was renamed to .yield().

Hi there!

Sorry for getting back to you so late, I felt like writing a longer response and did not find the time for that yet.

First of all, glad to hear you like the plugin! 🎉

Currently, the docs are written in a way that has worked out well in other parts of the docs.

  1. Simplify things so much that it is easy to use the library without thinking too much about it.
  2. Reiterate on the concepts and explain what stands behind them.

This is something that you will find in many other places on https://grammy.dev and I went with the same structure here, too.

However, for some reason, it seems like the attempt to cover up the complexity is a poor one. I have received this feedback in different forms already, so when you say

Great documentation!

then I don't really agree yet. We can do better.

Instead, I think we should interleave the two concepts. It can look something like this:

  1. Show very simple code for one thing.
  2. Explain how it works.
  3. Repeat for the next thing.

So basically, I would like to get rid of the section How It Works and put the same information everywhere throughout the article.

The docs were written when the plugin was very new, and no one really had a lot of experience with it. Since it is a novel abstraction that isn't found anywhere else, even I as the author of the plugin did not really know what would be the important things to talk about. Now that we all have a better understanding of it, I can write a better version of it. No idea when it will happen, but it's rather high up on my list currently :)

Regarding yield: there are two sides to it. It definitely feels like the conversation yields updates all the time, and at some point this method was actually called yield (but I reverted those changes before even committing them). You already named the reasons in favour of such a rename, so let me give you the reasons for why it wasn't called yield in the end:

  1. People are going to look for “how to wait for a message” not “how to yield updates” so wait fits the expectations.
  2. yield implies that it has something to do with iterators. That is misleading. The point is to persist state during execution, and co-routines do not do that.
  3. APIs should be very simple so that people with any depth of understanding can read the code and know what's going on. Since yield is sort of one level closer to what's actually going on, it requires a better understanding of the things that are going on. This is undesired.

It is not a coincidence that I wrote

The wait method always yields a new context object representing the received update

and I'm happy that there are people like you who get the hint :)