@Josh-Cena moving our comments from #9263 back over here for issue discussion:

@Josh-Cena:
I don't get the motivation for this, TBH. How can something "happen to be thenable" without noticing? Does then have any other meaning other than interoperating with promises? JavaScript itself resolves thenables, so if your thenable is not supposed to quack like a promise you will shoot yourself in the foot everywhere, like returning it from an async function, etc.

@JoshuaKGoldberg:

How can something "happen to be thenable" without noticing?
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/e40cab53d9971a3e0b34e8fb18ece74240d9425d/types/bluebird/index.d.ts#L59: older projects that have polyfills which generally intentionally match something like Promises/A+.

@Josh-Cena:
The issue mentions node:test, but doesn't test() return a real promise?

@Josh-Cena:
the Node issue does, but our #8433 -> this PR isn't around that I don't think.

@Josh-Cena:
That issue doesn't mention a valid use case where you want a thenable to not be checked like a promise though. It just mentions polyfills but those are why we check thenables.

Ah, I think I see what you're asking for - any reason why we would want to not check a thenable at all? That's a good question.

I'd come into this assuming it was a generally-desirable feature, but now I'm questioning that. Maybe now that we have the allowForKnownSafePromises option from #9186 we don't need this anymore?

Putting this issue into evaluating community engagement. If anybody can provide a real use case for not checking Thenables by default, please do - that would help us to justify adding #9263's checkThenables option. ❤️

Thanks for pressing @Josh-Cena!

The reason for not checking Thenable by default is that anything with a .then(res, rej) method could be awaited correctly. However, the spec is quite lax on purpose to allow for Thenable that are not Promise/A+ compatible.

If an object isn't a promise or promise-like, but it just has a .then() method, then it's already totally custom and not a Promise. Why Promise rule should apply?

I'm not sure I understand the comment as the two paragraph say opposite things.

• The first part says anything with then can be awaited—that's true. This means you fully expect then to be used in the promise resolution process.
• The second part says it's "totally custom and not a Promise". Then why do you expect it to be awaitable?

Put another way, either you have something that quacks like a promise and it's a bug to not await it, or it's not supposed to be a promise and it's a bug that await is not an identity operation on it.

The only valid use case I can think of is if you treat then like an operator overload hook for custom await behavior, but I have yet to see that being done.

We use it in Fastify quite extensively. In fact, we cannot write correct types for Fastify because of typescript-eslint. Your users would start plastering all their code of unneeded await and fill our issue tracker with question regarding this.

We have to resort to tricks like this:

https://github.com/fastify/fastify/blob/dda569960ef2d0961a887ead0e63f4e8671a920e/types/instance.d.ts#L173

instead of typing a .then() method into FastifyInstance.

@mcollina what would be useful for us here and in fastify/fastify#5506 (comment) would be: why does the FastifyInstance interface have a .then()? Can you share more on the design decision there?

For example, is it intentional that someone would await a value of type FastifyInstance? If so, why? If not, why the .then()?

(to be clear: not imparting judgement on the code, just trying to understand the context)

Fastify has a rich plugin system, and all plugins load asynchronously.

We support an API like:

const app = fastify()
await app
  .register(myPlugin1)
  .register(myPlugin2)
  .register(myPlugin3)

This is also supported:

const app = fastify()
app.register(myPlugin1)
app.register(myPlugin2)
await app.register(myPlugin3)

Adding the await will call .then(), which will, in turn, start the plugins' loading.

Note that awaiting the instance is entirely optional:

const app = fastify()
app.register(myPlugin1)
app.register(myPlugin2)
app.register(myPlugin3)
await app.listen({ port: 3000 })

There are also other methods on the app:

const app = fastify()
app.get('/', ...).post('/', ...)

await app.listen({ port: 3000 })

The FastifyInstance type does not have a .then() method because of the lack of this rule (and not being enabled by default).
According to no-floating-promises, if we added a .then() function, we would need to:

const app = fastify()
await app.get('/', ...) // or void app.get('/', ...)
await app.post('/', ...)

await app.listen({ port: 3000 })

This would work, but it adds unnecessary overhead and worsens the framework's DX.

Note that our Reply object has a then() method that will resolve when reply.send() is called. This allow us to support:

const app = fastify()

app.get('/simple', async function (req, reply) {
  return { hello: 'world' }
})

app.get('/complex', async function (req, reply) {
  // calling `.then()` here is incorrect, as it will wait for the response to be sent
  // however no-floating-promises forces the user to add a `void`
  void reply.header('x-foo', 'bar')
  setImmediate(() => {
    // Note that this break the promise chain
    reply.send('something')
  })
  return reply // this will can reply.then()
})

app.get('/empty', async function (req, reply) {
  // this will return an empty response
})

I see. So you have a chaining API where each chained operation can be optionally awaited, but usually they are not necessary. Yes it would be tricky to design such checks. Won't the allowKnownSafePromises option fix this case though?

Given that the rule is called no-floating-promises not no-floating-thenables (compared to await-thenables), I also don't have very strong opinions to not check thenables by default, given non-promise thenables are probably rare these days.

Won't the allowKnownSafePromises option fix this case though?

This would need to be enabled by the end users, and I would rather prefer if this wasn't needed at all. None of these are promises after all, and adding custom configs in every project is annoying.

Sure, I see the reasoning there. I think only checking native promises would be the least surprising behavior. promise-function-async has an option called allowedPromiseNames and maybe we can implement that instead of allowThenables? Thoughts @JoshuaKGoldberg?

Also noting @mcollina's comment from fastify/fastify#5506 (comment) regarding FastifyInstance:

And/or, why does it have a .then()?

It does have a then() so that it start loading all plugins. We are not typing it because otherwise every single method in Fastify would need to be awaited according to typescript-eslint and users have been complaining about that. In other terms, a user can do await app.register(myPlugin) instead of app.register(myPlugin); await app.ready(). It's syntactic sugar that improves the developer experience.

I would rather add a .then() method to FastifyInstance, but the amount of work that users of typescript-eslint would need to do will be substantial.

Essentially, we are accommodating typescript-eslint users.

only checking native promises would be the least surprising behavior

Agreed, which is what I see this issue as trying to approach. I'm also wondering if instead of an "all or nothing" boolean option value, we should turn checkThenables into an enum over:

• 'always': any value of a type with a .then()
• 'strict': any value of a type with a .then(resolve, reject) per the Promises/A+ spec
• 'never': never check on structural typing

...where maybe the recommended-type-checked config has it set to 'strict', and strict-type-checked config has it set to 'always'?

We're going to have to think on this a bit. Context: as with node:test (nodejs/node#51292) and RTK (reduxjs/redux-toolkit#4101), the way these frameworks are exposing Promises is counterintuitive to how I've understood their accepted userland semantics. These are roughly the only three cases I think we've seen in the wild of frameworks intentionally using Thenable-returning functions for something other than "call me to start some work that needs to be awaited". Part of the question for us is, how do we balance the needs of these not-typical-idioms vs. giving the intended effects for typical-idioms code?

promise-function-async has an option called allowedPromiseNames and maybe we can implement that instead of allowThenables?

That's what allowForKnownSafePromises is, no?

Note that .then(resolve, reject) would not fix us (#8433 (comment)). In other words, .then(resolve, reject) cannot guarantee Promise/A+ spec. You'll likely have to use a different type for those edge cases.

Yeah, I don't think "always" / "strict" makes sense. Fastify is not "accidentally-thenable", it's very much on purpose. No one is going to implement then "by accident" and not expect it to work with await.

Ok so coming back to this: what is the consensus here? I'm:

• Still in favor of making checking thenables an opt-in a la feat(eslint-plugin): [no-floating-promises] add checkThenables option #9263
• No longer suggesting we make it more granular than an always/never boolean

Also confirmed with @Josh-Cena 1:1 that we're good to continue. Great! 🚀