Promise#finally's callback doesn't accept Promise<void> callback, unlike Promise#then

[blair]

lib Update Request

Configuration Check

My compilation target is ES2018 and my lib is the default.

Missing / Incorrect Definition

Promise#finally

Sample Code

export async function closeDbConnection(): Promise<void> {
    return Promise.resolve();
}

Promise
    .resolve()
    .finally(closeDbConnection);

The suggested change is changing lib/lib.es2018.promise.d.ts from

finally(onfinally?: (() => void) | undefined | null): Promise<T>

to

finally(onfinally?: (() => void | PromiseLike<void>) | undefined | null): Promise<T>

The current typing, without the PromiseLike, suggests that the runtime will not wait for the onfinally callback to resolve if it's a Promise, however, it does. This can be seen in the console when test changes from pending to 123.

> test = Promise.resolve(123).finally(() => new Promise(resolve => setTimeout(resolve, 5000)))
Promise { <pending> }
> test
Promise { <pending> }
> test
Promise { <pending> }
> test
Promise { <pending> }
> test
Promise { <pending> }
> test
Promise { <pending> }
> test
Promise { <pending> }
> test
Promise { 123 }

Additionally, for consistency between #then and #finally, the current onfinally definition doesn't match Promise#then in lib/lib.es5.d.ts where onfullfilled returns TResult1 | PromiseLike<TResult1>.

interface Promise<T> {
    /**
     * Attaches callbacks for the resolution and/or rejection of the Promise.
     * @param onfulfilled The callback to execute when the Promise is resolved.
     * @param onrejected The callback to execute when the Promise is rejected.
     * @returns A Promise for the completion of which ever callback is executed.
     */
    then<TResult1 = T, TResult2 = never>(onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): Promise<TResult1 | TResult2>;

Documentation Link

https://tc39.es/ecma262/#sec-promise.prototype.finally

Suggests that .finally() is implemented similar to .then(result => Promise.resolve(onFinally()).then(result)).

[andrewbranch]

The current typing, without the PromiseLike, suggests that the runtime will not wait for the onfinally callback to resolve if it's a Promise, however, it does.

I don’t think it’s really fair to read runtime semantics from the type signature, particularly when void is involved. void in a return type position is special, because functions returning anything are assignable to functions returning void. For that reason, you should think of void as closer to unknown than undefined. Your sample code successfully type checks because of this rule, so just to clarify, are you proposing a typing change for purely documentative purposes?

Additionally, for consistency between #then and #finally, the current onfinally definition doesn't match Promise#then in lib/lib.es5.d.ts where onfullfilled returns TResult1 | PromiseLike<TResult1>.

That definition is written that way because it’s actually relevant to the inference for TResult1. Adding PromiseLike<void> to the return type for onfinally has no effect on inference since it references no type parameters.

[blair]

@andrewbranch thanks for looking. eslint with @typescript-eslint/eslint-plugin and @typescript-eslint/parser issues a warning for this code. See #38752 please for the code that generates the warning. Are you suggesting that eslint would then be changed to not warn?

[andrewbranch]

I see. I am saying that the TypeScript typings are correct, because you can’t prove that there’s any problem with them without introducing a third-party tool which introduces its own semantic opinions. TypeScript is, more or less strictly, and among other things, in the business of ensuring that you write code that satisfies type definitions, and functions that return promises clearly satisfy the parameter type for Promise#finally, because we do not issue an error when you write one there. If you write Promise.resolve().finally("hello"), we issue an error because that’s incorrect. If you write Promise.resolve().finally(() => Promise.resolve()), we do not issue an error. This is the method by which you make assertions about the correctness of type definitions. You can’t claim that a set of type definitions are incorrect without showing either (a) code that should error but doesn’t, or (b) code that doesn’t error but should. Third-party lint rules and human interpretation are not substitutes for those assertions. In this particular case, that ESLint rule either misunderstands or intentionally extends TypeScript’s assignability rules for void-returning functions.

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[adit-hotstar]

@andrewbranch Foreword: In the following paragraphs I'm not trying to be rude or disrespectful. I reviewed my comment and made sure that I'm as respectful as possible. Nevertheless, please forgive me if it seems as if I'm being rude or disrespectful. That is not my intention. My only intention is to engage in an honest discussion with an open mind.

I don’t think it’s really fair to read runtime semantics from the type signature, ...

I disagree. I think it's perfectly reasonable to read runtime semantics from the type signature. A type signature should inform the user about what the function does. For example, when I see a function of the type <A, B>(a: A, b: B) => A I can be confident that, sans side effects and type-unsafe shenanigans, the function simply returns its first input.

...particularly when void is involved.

Again, I disagree. When a higher-order function says that the return type of one of its callbacks is void, I expect the higher-order function to ignore the return value of that callback. For example, Array#forEach ignores the return value of its callback. Hence, it makes sense for its callback to have the return type void.

However, Promise#finally doesn't ignore the return value of onfinally. Hence, it's dishonest to say that the return type of onfinally is void. When a person who doesn't know any better reads the type Promise<T>.finally(onfinally?: (() => void) | undefined | null): Promise<T> they might incorrectly assume that the return value of onfinally would be ignored. I imagine that they'd be surprised when Promise.resolve(10).finally(() => Promise.reject('ten')).then(console.log) doesn't log anything to the console.

void in a return type position is special, ...

Yes, which is why we should be careful not to abuse it. In my humble opinion, void is being abused in the return type of onfinally to avoid writing a longer return type. Although using void as the return type of onfinally results in a shorter type, yet as I showed above it's a dishonest type.

Your sample code successfully type checks because of this rule, ...

Just because a program type checks it doesn't imply that the type definitions are correct. For example, const x: any = 10; const y: string = x; type checks but y is a number, not a string. Also, just because a program type checks it doesn't imply that the type definitions are accurate. For example, const x: unknown = 10 type checks but is inaccurate because we know for a fact that x is a number. Finally, just because a program type checks it doesn't imply that the type definitions are honest. For example, Promise#finally promises that it will ignore the return value of onfinally, but it doesn't.

... so just to clarify, are you proposing a typing change for purely documentative purposes?

I can only speak for myself, not for the OP, but yes I'm in support of a typing change for purely documentation purposes. Types are meant for documentation just as much as they are meant for ensuring type-safety. TypeScript can't guarantee type-safety because of stuff like any. At least let us have types that are good for documentation purposes by ensuring that they aren't dishonest.

That definition is written that way because it’s actually relevant to the inference for TResult1. Adding PromiseLike<void> to the return type for onfinally has no effect on inference since it references no type parameters.

How about the following type which references a new type parameter B?

/**
 * Represents the completion of an asynchronous operation
 */
interface Promise<T> {
    /**
     * Attaches a callback that is invoked when the Promise is settled (fulfilled or rejected). The
     * resolved value cannot be modified from the callback.
     * @param onfinally The callback to execute when the Promise is settled (fulfilled or rejected).
     * @returns A Promise for the completion of the callback.
     */
    finally<B>(onfinally?: (() => B | PromiseLike<B>) | undefined | null): Promise<T>
}

This is actually the most accurate and honest type for Promise#finally. It's accurate because it matches what Promise#finally really does implementation-wise. In fact, the finally function from the Control.Exception package in Haskell also has a similar type, IO a -> IO b -> IO a. It's honest because it tells the reader that,

1. they can return a value or a promise-like object from the onfinally function,
2. the result of the onfinally function might be used by Promise#finally,
3. if the final promise resolves then it will resolve with the value of the original promise.

You can’t claim that a set of type definitions are incorrect without showing either (a) code that should error but doesn’t, or (b) code that doesn’t error but should.

Forgive me for nitpicking here but I think you meant “(b) code that does error but shouldn't.” Anyway, I agree with you that the current type definition of Promise#finally is indeed correct because it doesn't have (a) any false positives, or (b) any false negatives.

Nevertheless, although it is correct yet it is neither accurate nor honest. It's not accurate because there is a subtype of Promise<T>.finally(onfinally?: (() => void) | undefined | null): Promise<T> which is also correct. It is not honest because it promises to ignore the return value of onfinally even though it doesn't.

Third-party lint rules and human interpretation are not substitutes for those assertions.

The aforementioned typescript-eslint/no-misused-promises rule might be wrong here, and human interpretation might be subjective. However, the current type definition of Promise#finally is objectively inaccurate and dishonest. At the end of the day, it's not for me to decide whether or not the type definition of Promise#finally should be updated. However, I do hope that I have convinced you that there is a more accurate and honest type definition for Promise#finally which is also correct.

[andrewbranch]

Thanks for a well-thought-out response. To be totally honest, I was extra critical of this particular issue because the OP had already posted the same issue in the past and it had been closed as external, and this issue didn’t even mention the original until I followed up. We really can’t have people opening their own duplicates in hopes of eventually getting an answer they like better. This whole discussion ideally should have happened over at #38752. But since you bring up some points worth addressing, here we go.

when I see a function of the type <A, B>(a: A, b: B) => A I can be confident that, sans side effects and type-unsafe shenanigans, the function simply returns its first input.

No, you definitely can’t—all you can be confident about is that the type of the value returned will be assignable to the type of the first parameter. It could be a different value of the same type as the parameter, or it could be a subtype.

A lot of your arguments are about human interpretation about how we should interpret types, and programs that type-check, beyond their immediately observable consequences. These are reasonable arguments, for sure, but let’s not call them objectively true as you conclude. But let me address your proposal before getting into matters of opinion:

How about the following type which references a new type parameter B?

This is what we call a “useless type parameter” because it doesn’t correlate anything. In fact, a smart linter will complain about this, though in practice it may be fooled by the fact that you’ve used it twice in a single union—you’d have to check. At any rate, the purpose of type parameters is to link two or more parts of the structure of a type. In your earlier example that I nitpicked, you linked the type of the first parameter of a function with its return type. When a type parameter is used in only one part of the structure of a type, it’s useless and should be removed. We would not consider adding this pattern to our library files.

Back to opinions and philosophies:

void is a terrible type, and I don’t like what it implies here. I think you’re right about what people would assume just from looking at that signature, whether or not they are justified in that assumption. Yes, types have some value as documentation, but we have generally been opposed to writing types in such a way that is purely documentary and has no observable effects on the type system. An example that often comes up is stuff like type Color = "red" | "green" | "blue" | string. Perhaps your library has a special way of treating "red" and "green" and "blue" but also accepts arbitrary strings that it will interpret in some way. You are allowed to write this type, but we do not retain any meaning from the union. It collapses to string immediately. We get proposals from time to time to preserve unions like this so that the type can be ferried around with all its documentary value, but ultimately I think we believe that “types as documentation” is kind of secondary to the type-checking, or at least, you can’t use types to document behavior that we can’t actually analyze in the type system. There has to be a technical meaning as the basis for the interpretive meaning. This explains why your claim isn’t quite right:

void is being abused in the return type of onfinally to avoid writing a longer return type

It’s not the length of the type that we care about; it’s more the density. We don’t want any meaningless parts; that is, we don’t want anything that doesn’t participate in type checking.

So the question becomes, is there a meaningful and accurate way to write this type while avoiding the pitfalls of void? I would be amenable to replacing void with unknown here. We have often argued that unknown is the most appropriate type for callbacks that are currently typed as void-returning, and I think that’s particularly applicable here, since as you say, the return value is meaningful to the runtime, while it’s not meaningful to the type system. We normally use any rather than unknown in our lib files as a backward compatibility measure, though I think they would have the same effect here. I can’t guarantee it would be accepted, but I think we could consider a PR changing this void to any or unknown. I also suspect that would silence that lint rule, though I cannot emphasize enough how little that factors into my willingness to change this. To express openness to this, I feel I also have to caveat that external lint rules are not a reason to change the compiler, and duplicating old rejected issues is not a good way to get your issue reconsidered! 😐

[blair]

As the OP, apology for not linking to the original bug #38752.

Regarding opening this bug, I didn't think the original bug (#38752) was given close attention that it needed. After it was marked external, I replied and a bot closed it, I didn't see a way to reopen it to continue the conversation. I then emailed the issue to some TypeScript more-expert-than-me colleagues at work and there was consensus on the issue was fair. So I reopened this one without any references to typescript-eslint to keep it focused on TypeScript itself.

I agree, making a new bug to continue the discussion isn't good. I would like to request a way to reopen bugs or have a second look at them. Any suggestions? How would I do that in the future?

Thanks for reconsidering this topic.

[adit-hotstar]

when I see a function of the type <A, B>(a: A, b: B) => A I can be confident that, sans side effects and type-unsafe shenanigans, the function simply returns its first input.

No, you definitely can’t—all you can be confident about is that the type of the value returned will be assignable to the type of the first parameter. It could be a different value of the same type as the parameter, or it could be a subtype.

No, it actually can't be a different value of type A, much less a different value of the subtype of A. This is because A is a type parameter which could be instantiated with an arbitrary type. You have no way of conjuring a different value of type A because you don't know what type A will be instantiated with.

You may think that it is possible to narrow the type of A and thereby return a different value. However, TypeScript doesn't allow that either. See the following example in the TypeScript Playground. I encourage you to play with this example to convince yourself that TypeScript will indeed never allow you to return anything but the first parameter.

const foo = <A, B>(a: A, _b: B): A => {
    if (typeof a === 'number') {
        // Type 'number' is not assignable to type 'A'.
        // 'A' could be instantiated with an arbitrary type which could be unrelated to 'number'.
        return 10;
    }

    if (a instanceof Date) {
        // Type 'Date' is not assignable to type 'A'.
        // 'A' could be instantiated with an arbitrary type which could be unrelated to 'Date'.
        return new Date();
    }

    return a;
};

The only way we can return a value other than the first parameter is if we disable the type checker by type casting the return value to any. However, I think I covered my bases there when I said, “I can be confident that, sans side effects and type-unsafe shenanigans, the function simply returns its first input.”

A lot of your arguments are about human interpretation about how we should interpret types, and programs that type-check, beyond their immediately observable consequences. These are reasonable arguments, for sure, but let’s not call them objectively true as you conclude.

I only concluded that the current type definition of Promise#finally is objectively (a) inaccurate, and (b) dishonest. It's inaccurate because you can assign a more specific type to the return value of onfinally without making the type of Promise#finally incorrect, using the definition of correctness that you mentioned. It's dishonest because the type of Promise#finally promises that it will ignore the return value of onfinally when in fact it doesn't do so.

Both of these are objective facts. They aren't open to human interpretation. The type B | PromiseLike<B> for an arbitrary type B is objectively more specific than void. On the other hand, ignoring the return value of void-returning callbacks is not my own subjective view. This interpretation of void-returning callbacks is stated by the TypeScript wiki itself:

Another way to think of this is that a void-returning callback type says "I'm not going to look at your return value, if one exists".

— TypeScript Wiki FAQ: Why are functions returning non-void assignable to function returning void?

This is what we call a “useless type parameter” because it doesn’t correlate anything.

Actually, it does correlate the two sides of the union. If the left-hand side of the union is B then the right-hand side of the union must be PromiseLike<B>, and vice-versa. That's a correlation. It might not be correlating the inputs of Promise#finally with its output, but it's still a correlation. You can't substitute B with void or unknown without losing this correlation.

However, even if B didn't correlate anything, it's still not useless. You can instantiate B to an arbitrary type. For example, Promise.resolve(10).finally<string>(() => 'ten') will type check but Promise.resolve(10).finally<string>(() => true) won't. I can see this being useful to pin down specific types for robustness, for example when writing unit tests.

At any rate, the purpose of type parameters is to link two or more parts of the structure of a type.

I disagree. Consider a function of the type <A>(array: A[]) => number. Here, the type parameter A doesn't link anything. However, that doesn't mean that it's useless. In fact, I would argue that it's more useful than a type like (array: unknown[]) => number because (a) it can be instantiated to a specific type for robustness, for example when writing unit tests, and (b) it is more readable to functional programmers like me, although I'll concede that this point is subjective.

When a type parameter is used in only one part of the structure of a type, it’s useless and should be removed.

This is a very subjective statement. To a functional programmer like me, unknown and never are useless because both can be replaced by type parameters. In fact, to me type parameters are more useful than unknown and never because (a) they can be instantiated with specific types for robustness, and (b) they are more readable.

We would not consider adding this pattern to our library files.

Fair enough. At the end of the day, TypeScript is not my project. However, I would like to state that I see several advantages of using type parameters instead of unknown and never, and no disadvantages.

There has to be a technical meaning as the basis for the interpretive meaning. This explains why your claim isn’t quite right:

void is being abused in the return type of onfinally to avoid writing a longer return type

It’s not the length of the type that we care about; it’s more the density. We don’t want any meaningless parts; that is, we don’t want anything that doesn’t participate in type checking.

On the contrary, if you don't want any meaningless parts then you should use a type parameter instead of unknown. The unknown type literally doesn't participate in type checking because every type can be assigned to unknown. On the other hand, if you use a type parameter then you can either (a) instantiate it and then let the type checker continue checking, or (b) let the type checker infer the type of the parameter and then continue checking. In both cases, type parameters participate in type checking more than unknown does.

Also, there are technical bases for the type Promise<T>.finally<B>(onfinally?: (() => B | PromiseLike<B>) | undefined | null): Promise<T>. First, the type parameter correlates the left-hand side of the union with the right-hand side. If you substitute B for void or unknown then this correlation will be lost. Second, you can instantiate the type parameter B to an arbitrary type. This is something you can't do with void or unknown. Overall, I would say that B | PromiseLike<B> is more meaningful and more dense than both () => unknown and () => unknown | PromiseLike<unknown>.

I would be amenable to replacing void with unknown here.

The type () => unknown is not much better than () => void. It would only serve to confuse the reader, as they wouldn't understand why the return type is unknown. The way I see it, unknown should only be used for types that you literally don't know. For example, when fetching data from a server or using a third party library which doesn't export types. We actually do know the exact return type of onfinally. It's B | PromiseLike<B> for some arbitrary type B.

We have often argued that unknown is the most appropriate type for callbacks that are currently typed as void-returning, and I think that’s particularly applicable here, since as you say, the return value is meaningful to the runtime, while it’s not meaningful to the type system.

Well, if you use the return type B | PromiseLike<B> then it will be meaningful to both the runtime as well as the type system. In particular, you can either instantiate B to a specific type that the type system can check or you can let the type system infer B. In both cases, the return type is meaningful to the type system.

We normally use any rather than unknown in our lib files as a backward compatibility measure, though I think they would have the same effect here.

You know what else is backward compatible? Type parameters. You get all of the benefits of not using any without actually using unknown, which as we all know is not backward compatible.

I also suspect that would silence that lint rule, though I cannot emphasize enough how little that factors into my willingness to change this. To express openness to this, I feel I also have to caveat that external lint rules are not a reason to change the compiler, and duplicating old rejected issues is not a good way to get your issue reconsidered! 😐

I don't care about external lint rules either, so we're on the same page here. 😄

[andrewbranch]

I would like to request a way to reopen bugs or have a second look at them. Any suggestions? How would I do that in the future?

Just comment on the closed bug. We don’t unsubscribe from closed issues, and if there is a reason to reopen them or reengage with them, we do—as is happening here.

You have no way of conjuring a different value of type A because you don't know what type A will be instantiated with.

I think you’re forgetting that you have ways of making plausibly correct subtypes of objects in JavaScript. It’s true that if you wind up with primitives you’re kind of stuck. https://www.typescriptlang.org/play?#code/MYewdgzgLgBAZiEBGGBeGAeAggGhgIQD4AKAQwC4ZcYAjS-ASkqzUJgG8AoGHmASzgwyMAGQjao8VACeABwCmIQaTSp0AIhA0AVvOBR1kmDIVKJajVt371DDt16OATvKgBXJ2BgB5HXqgAdKQQEHwA5mDE7AC+eDR4pAwA3A480akwLu6eMKQp6UA

You can also do some arguably correct things with user-defined type guards, which are not checked for accuracy, but are not wrong and are less suspicious than casting to any. If you have some pattern of runtime reflection that TypeScript can’t analyze, but you’ve convinced yourself of its safety and its utility, I don’t think I’d call that “shenanigans” anyway; I’d call it a limitation of our compiler.

I guess we’ll have to agree to disagree about what type parameters are for. I won’t be monitoring this thread anymore as the discussion has been exhausted of anything useful to resolving the original issue.

[jedwards1211]

@andrewbranch are there any concrete downsides to having PromiseLike in the signature though? I don't see signs of an internally consistent philosophy to favor just void when the promise value isn't needed for inference.

There are plenty of other builtin lib defs for callbacks that return void | PromiseLike<void>, for instance TransformerFlushCallback:

interface TransformerFlushCallback<O> {
    (controller: TransformStreamDefaultController<O>): void | PromiseLike<void>;
}

If there were a way in TS types to declare that a callback whose return value is ignored must be synchronous then ESLint could be smarter about this rule, but there's no way to declare that in TS types AFAIK.