Checking existence of entry in Record<string, Promise<string>> always yields "This condition will always return true since this 'Promise<string>' is always defined"

[rasmusvhansen]

🔎 Search Terms

This condition will always return true since this 'Promise' is always defined
Record promises

🕗 Version & Regression Information

• This changed between versions 4.3.5 and 4.4.4

⏯ Playground Link

https://www.typescriptlang.org/play?#code/MYewdgzgLgBADgJxAWwJYQKYGECGwAWGAXDAEoagIAmAPNAqmAOYA0MACkmpnVA8wD4BMALwwA3gF8A3AChZAMwCuYYFFTgYTDFE4p0GABQBrDAE8S9RkwCUE2TEcxUCmIcT7MuAhgDapswBdO3EHJ3CEHSUEMBgAcgAxEBUqOLlwyTDHSKho2LiAORBYBWSwVLlMoA

💻 Code

const promiseCache: Record<string, Promise<string>> = {};

function getPromise(key: string) {
    // Error shows up here: "This condition will always return true since this 'Promise<string>' is always defined.(2801)
    // input.tsx(4, 9): Did you forget to use 'await'?"
    if (promiseCache[key]) {
        return 'Found';
    }
    return 'Not found';
}

🙁 Actual behavior

It should be clear that promiseCache[key] is not always defined, and that this has nothing to do with it containing promises or a missing await. It is just a record of objects which happen to be promises

🙂 Expected behavior

There should not be an error, since

if (promiseCache[key]) {
    return 'Found';
}

Is a valid check in this case

Additional information about the issue

No response

[MartinJohns]

This is working as intended. You need to enable the noUncheckedIndexedAccess compiler option. Without the option each property accessed is assumed to be the type Promise<string>, and Promise<string> is always truthy. Alternatively you can change the type to Promise<string> | undefined.

[rasmusvhansen]

Yes enabling noUncheckedIndexedAccess fixes it.
But is that really the intention that in noUncheckedIndexedAccess is false, it is disallowed to check after accessing an index?

If that is the case, why is it not consistent so an error is also produced if the cache was typed as

const cache: Record<string, string> = {};

This produces no error even though it should be the same if it were consistent:

https://www.typescriptlang.org/play?ts=4.4.4#code/MYewdgzgLgBADgJxAWwJYQKYGECGwAWGAXDAEoagIAmAPNAqmAOYA0M9jTAfDALwwBvAL4BuAFBiAZgFcwwKKnAwmGKAAUkaTAAoA1hgCeJDswCUgsTCsxUkmNsQp02PIQDa+gwF1zAy9YCEVWkEMBgAcgAxEFkqcPEAoX8rIKgQsPCAORBYSRiwOPEkoA

[fatcerberus]

Because strings can be falsy (""). Object types like Promise can’t.

[rasmusvhansen]

Because strings can be falsy (""). Object types like Promise can’t.

No, that is not it

const cache: Record<string, object> = {};

function getPromise(key: string) {
    if (cache[key]) {
        return 'Found';
    }
    return 'Not found';
}

yields no error as well. There seems to be a special case for promises which is not consistent with "normal" objects.

https://www.typescriptlang.org/play?ts=4.4.4&ssl=9&ssc=1&pln=1&pc=1#code/MYewdgzgLgBMCGwAWBTAXDASi0AnAJgDzS4CWYA5gDQwgBGAVjlAHwwC8MA3gL4DcAKAEAzAK5hgUUuBgUUUAAq4QAW1IQUACgDWKAJ4YS5CgEpuAmJZilhMTQmQoA2rr0BdM1wtWfueaNwwGAByADEQcXxgwR8eb0s-KACg4IA5EFhhCLAowTigA

[fatcerberus]

That should be an error, yeah. Either way, the root cause here is still that Record<string, T> produces T when accessed, not T | undefined, and the solution is to either enable noUncheckedIndexedAccess or explicitly add undefined to the type.

[rasmusvhansen]

Without the option each property accessed is assumed to be the type Promise<string>, and Promise<string> is always truthy. Alternatively you can change the type to Promise<string> | undefined.

@MartinJohns, why is the behaviour different when having a Record<string, object> shouldn't it behave the same if what you wrote above is the cause of the error?

@fatcerberus, yes. Enabling noUncheckedIndexedAccess is a solution. But typing a record as Record<string, T | undefined> seems like a weird hack since that is the nature of a record that you risk getting undefined when accessing it.

However, it still seems like a bug to me that the behaviour is different just because the record contains a special type of object (Promise<T>)

[fatcerberus]

It’s not just because it’s a “special type of object”, but because it’s an object type at all being checked for truthiness when objects are known by the compiler to always be truthy. (The fact that it doesn’t show the error for object is probably a bug.)

that is the nature of a record that you risk getting undefined when accessing it.

Yes, but index signatures such as the one on Record<string, T> are unsound for convenience; any property you access is assumed to always exist and be the correct type. noUncheckedIndexedAccess was added later to rectify this, but it breaks a lot of idiomatic code so it’s not enabled by default.

The same thing applies to accessing arbitrary indices of an array, for what it’s worth.

[rasmusvhansen]

@fatcerberus, ok, so what you are saying is that the bug is that the error only shows for Promise and not any other object type (including user defined types)?

I am not entirely sure I agree, but I guess that is a design decision for the TS team which way to go. But we agree that it should behave the same regardless of whether the values in the record are promises or any other object type, right?

In any case, the hint about Did you forget to use 'await'?" is misleading, since this has nothing to do with the value being a promise.

Example with user defined type (instead of object):

https://www.typescriptlang.org/play?ts=4.4.4#code/C4TwDgpgBAsiAq5oF4oG8B2BDAthAXFAM7ABOAlhgOYC+A3AFCiSwJIBMUqACqQPY5yRCAB4SFagD5GDAMZ8MJKLKyyAFgSgAlCPNIATMWUpUANK0SRJXdPQYMAZgFcMs4OQVQqEYLwFCIAAoAawgQQnETAEp0Bih4qHIHKECVdQgAbVCQAF0YtDiEotIfJ1IMKAByADE+F31KxiKaQviS4DKKyoA5PmAoBzqMBsYWoA

[MartinJohns]

why is the behaviour different when having a Record<string, object> shouldn't it behave the same if what you wrote above is the cause of the error?

That's a long standing design limitation involving object resulting in unsoundness. Basically object is always assumed to be truthy, even if falsy primitives are assignable to it. See also #31464. Keep in mind that TypeScript is not designed to have a fully sound type system.

Can't say anything other than "this is working as expected" and that enabling noUncheckedIndexedAccess is the intended solution for this. WiseCerberus already mentioned why this is not enabled by default. I'm sure if I look a minute or two I can find an exact duplicate of this issue.

[MartinJohns]

Duplicate of #55852, which is marked "Working as Intended". Not 100 % exact, but essentially the same issue: assuming record access could yield in undefined.

[rasmusvhansen]

That's a long standing design limitation involving object resulting in unsoundness. Basically object is always assumed to be truthy, even if falsy primitives are assignable to it. See also #31464. Keep in mind that TypeScript is not designed to have a fully sound type system.

@MartinJohns
Using object in the example was a bad idea on my part.

Please see this example. This example does not allow assigning falsy primitives yet still is inconsistent.

const key: string = 'someKey';
const a: Record<string, Promise<string>> = {};
const b: Record<string, {a: string}> = {};
// errors here with message about using await
if (a[key]) console.log('found');
// but not here
if (b[key]) console.log('found')

Would you also say that is working as intended?
If yes, then the hint about Did you forget to use 'await'?" is quite misleading.

If not, would it be better to open a new issue describing the inconsistent behaviour in the code above?

https://www.typescriptlang.org/play?ts=4.4.4#code/C4TwDgpgBAsiAq5oF4oG8B2BDAthAXFAM7ABOAlhgOYC+A3AFCiSwJIBMUqACqQPY5yRCAB4SFagD5GDAMZ8MJKLKyyAFgSgAlCPNIATMWUpUANK0SRJXdPQYMAZgFcMs4OQVQqEYLwFCIAAoAawgQQnETAEp0Bih4qHIHKECVdQgAbVCQAF0YtDiEotIfJ1IMKAByADE+F31KxiKaQviS4DKKyoA5PmAoBzqMBsYWoA

[rasmusvhansen]

Duplicate of #55852, which is marked "Working as Intended". Not 100 % exact, but essentially the same issue: assuming record access could yield in undefined.

I disagree that this is a duplicate, since the issue I am (trying to) describe is the inconsistent behaviour when accessing these two records without noUncheckedIndexedAccess enabled. Sorry for not being very clear on that.

const a: Record<string, Promise<string>> = {};
const b: Record<string, {a: string}> = {};

[fatcerberus]

Oh, the error you're seeing is the "did you forget to use await" one, but yeah, that's intentional too. It's designed to catch this mistake:

const foo = getFooAsync();  // returns a promise
if (foo) {  // oops, always truthy but you probably just forgot to await the promise
    // do stuff
}

[MartinJohns]

I disagree that this is a duplicate, since the issue I am (trying to) describe is the inconsistent behaviour when accessing these two records without noUncheckedIndexedAccess enabled.

In that case it's also intentional, see #25330 (this also matches your changed between versions). This behaviour is irregardless of using a Record<> type - any truthy check of a Promise<> results in the error about a missing await.

With noUncheckedIndexedAccess enabled you don't get this error, because now you're not checking a Promise<T> anymore, you're checking a Promise<T> | undefined. It's just an unfortunate combination in this case. I don't see how the error message could be improved in a meaningful way, without special casing it to the compiler option disabled and accessing a record/property.

Sorry for not being very clear on that.

No sweat.

[rasmusvhansen]

Alright, I guess I hit an interesting edge case where I have not (yet) enabled noUncheckedIndexedAccess on the codebase and I actually want to verify that I get a hit in a cache of promises. A workaround for me is to do
if (!!cache[key]) to convert the Promise | undefined to a boolean

The hint about using await makes sense in most other cases and I agree that it would be probably be hard to improve the message for this special case.

Thanks for clearing that up for me 👍