Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise intersection with primitive results in wrong awaited type #48927

Closed
Azarattum opened this issue May 3, 2022 · 13 comments
Closed

Promise intersection with primitive results in wrong awaited type #48927

Azarattum opened this issue May 3, 2022 · 13 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Azarattum
Copy link

Bug Report

When we make an intersection of a promise with a primitive type like Promise<T> & T (where T is primitive (non-object)), the resulting type from await (Promise<T> & T) is wrong (still contains a promise). This is clearly a bug since Awaited<Promise<T> & T> and (Promise<T> & T).then(x => {}) behave correctly (the result is only T). The issue is only observed when using the await keyword.

The types arrangement might seem arbitrary, but it has a valid use case which is provided in the playground link below.

🔎 Search Terms

PROMISE & PRIMITIVE, PROMISE INTERSECTION, AWAITED TYPE, INCONSITENT THEN AND AWAIT

🕗 Version & Regression Information

Tested only versions available on playground!

  • This changed between versions 4.5.5 and 4.7.0-dev.20220503. 4.4.4 and lower are unaffected by this bug.

⏯ Playground Link

Playground link with relevant code

💻 Code

type A = Promise<number> & number;
const a = {} as A;

const b = await a;
type B = typeof b; // B should be "number", but it is "Promise<number> & number"!

a.then((c) => {
  type C = typeof c; // C is "number" (which is corrent)
});

type D = Awaited<A> // D is "number" (which is corrent)

🙁 Actual behavior

Type B still contains Promise<number> despite of the value being awaited.

🙂 Expected behavior

In the example above B has to be equal to C and D.

@Azarattum
Copy link
Author

#45350 this is probably related. However, as shown before, the Awaited itself works correctly.

@fatcerberus
Copy link

fatcerberus commented May 3, 2022

I don’t understand (even with your playground example) why you would ever write Promise<number> & number. This type represents a value that is simultaneously both a promise object and a primitive number, which is impossible.

@jcalz
Copy link
Contributor

jcalz commented May 3, 2022

The code in there seems to be using symbol, but there are no values which are both symbols and promise objects. Could you clearly articulate the use case in words?

Even better, could you show a working example of your use case at runtime? Your playground link uses {} as the runtime value which won't actually work at runtime. Like, forget the TS types for a bit, what kind of JS behavior are you trying to support?

@andrewbranch
Copy link
Member

It does seem theoretically wrong, but yeah, unless there is a real-world use case I doubt we’ll prioritize a change here.

@andrewbranch andrewbranch added the Bug A bug in TypeScript label May 3, 2022
@andrewbranch andrewbranch added this to the Backlog milestone May 3, 2022
@Andarist
Copy link
Contributor

Andarist commented May 3, 2022

I don't think this would be a hard fix - at the moment this is somewhat explicitly called out to be invalid in one of the comments:
https://github.dev/microsoft/TypeScript/blob/8f56f6b49d2fb282ff95df64f62dab2947004e95/src/compiler/checker.ts#L36317-L36321

Probably one of the required changes to make this work would be to tweak the related logic in the getPromisedTypeOfPromise here:
https://github.dev/microsoft/TypeScript/blob/8f56f6b49d2fb282ff95df64f62dab2947004e95/src/compiler/checker.ts#L36265-L36268

I could take a stab at this - if only somebody would confirm that this behavior needs to be adjusted. I also don't see though how this could be utilized in the real-world code so far.

@andrewbranch andrewbranch added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels May 3, 2022
@andrewbranch andrewbranch removed this from the Backlog milestone May 3, 2022
@andrewbranch
Copy link
Member

I did a quick experiment monkeypatching String.prototype.then to see if it would be called by Promise.resolve (or await, which uses the algorithm of the former). It did not. It’s pretty hard to follow but I think I found this in the spec: https://tc39.es/ecma262/#sec-promise-resolve-functions

  1. If Type(resolution) is not Object, then
    a. Perform FulfillPromise(promise, resolution).
    b. Return undefined.

The only way I can interpret string & Promise<string> is a value that is definitely the primitive string but also manages to satisfy the Promise interface structurally, which perhaps you could do with a horrifying regimen of monkeypatching. The spec and my experiment seem to agree that such a value (the stuff of nightmares) would in fact emerge unchanged from await. So the type is correct.

@Azarattum
Copy link
Author

Thank you all for your feedback.

@andrewbranch I don't think this issue should be closed. I just made a full example use case which roughly replicates what I'm trying to achieve in my project. I'm working on a custom asynchronous store for Svelte similar to this one or this one. The idea behind it is to abstract away API interactions, so the store can manage everything. To achieve that I'm using a data structure which I call RichPromise which is basically Promise<T> & T of anything (including primitives). This is achieved by constructing a special prototype chain which includes the original object's prototype, bound methods for primitives, overrides such things as [Symbol.toPrimitive] and valueOf, and adds then, catch, finally to mimic a promise behavior (for implementation details see enrich function in the example). Because the original prototype is included in RichPromise's chain enrich(Promise.resolve("2"), "1") instanceof String is true.

This has one caveat though. When we first look at out rich promise (before any api calls resolve), there is no data to be concatenated with a promise, so we pair it with loading symbol placeholder. This way the type becomes Promise<T> & (T | typeof loading). I choose to use a symbol, so we can infer the loading state by type with if (richPromise == loading) which works because of the [Symbol.toPrimitive] override. So, it is must have for primitive types to work correctly because the guard above only works if we use unique symbol, but not Symbol which makes sense. However as we do not have unique Symbol as an object there is no other way to solve it.

This whole approach was chosen to elegantly support 2 different rendering strategies in svelte templates:

<!-- Strategy 1 -->
<p>
  {#if $store != loading}
    {$store}
  {:else}
    <!-- Is shown only the first time -->
    Loading
  {/if}
</p>

<!-- Strategy 2 -->
<p>
  {#await $store}
    <!-- Is shown every time new data starts loading -->
    Loading
  {:then data}
    {data}
  {/await}
</p>

In the linked example I'm trying to mimic something similar to svelte stores without the framework. If you run the code, you'll see the showcases of the two strategies shown above:

Showcase loading strategy 1
Loading...
(3) ['super', 'important', 'data'] 'Data: super important data'
(3) ['super', 'important', 'data'] 'Data: super important data'
(3) ['some', 'changed', 'data'] 'Data: some changed data'

Showcase loading strategy 2
Loading...
(3) ['super', 'important', 'data'] 'Data: super important data'
(3) ['super', 'important', 'data'] 'Data: super important data'
Loading...
(3) ['some', 'changed', 'data'] 'Data: some changed data'
(3) ['some', 'changed', 'data'] 'Data: some changed data'

Note that in a real scenario the same data would not be rendered twice.

Anyway besides all of that, this is INCONSISTENCY (between then, await, Awaited) and should be considered a bug. Moreover, since this was introduced just recently it cannot be seen an intended behavior.

@Azarattum
Copy link
Author

Also, if we had a way to

data: A | B;
if (data != B) {
  data: A;
}

where B extends object. That would also solve my particular use case. But I would rather get the original issue solved for the sake of consistency.

If anyone knows if the snippet above is achievable (without changing an operator or using a function) please let me know.

Fun fact

I also tried to use document.all and %GetUndetectable() for prototyping a falsy object to a promise, so this would be possible:

{#if $store}
  {$store}
{:else}
  Loading
{/if}

{#await $store}
  Loading
{:then data}
  {data}
{/await}

But it was a little bit to far... And there were some problems with promises not being detected since typeof document.all returns undefined and not an object. So I decided to go ahead with the symbol approach.

@Azarattum
Copy link
Author

And even ignoring everything considered, it just makes sense for await Promise<number> & number to return number. Regardless of if anybody is going to use it or not...

@fatcerberus
Copy link

fatcerberus commented May 4, 2022

enrich(Promise.resolve("2"), "1") instanceof String is true

Then what you have is a String, not a string. It is literally impossible to construct a value of type Promise<string> & string.

C:\Users\fatce>node
Welcome to Node.js v18.0.0.
Type ".help" for more information.
> "foo" instanceof String
false
> typeof "foo"
'string'
> typeof new String("foo")
'object'

This distinction matters because:

const x: string = new String("foo");  // error TS2322
// Type 'String' is not assignable to type 'string'.

@andrewbranch
Copy link
Member

Indeed, what you actually have is objects, and you’re interoperable with primitives due to [Symbol.toPrimitive] and valueOf. What you want is #2361. String & Promise<String> awaited is String, so in the world of #2361 where that could be made compatible with string, your use case works.

@Azarattum
Copy link
Author

Azarattum commented May 5, 2022

@andrewbranch Yeah... I think you are right after all. I did use an Object(<value>) wrapper when constructing my promises. So the types are actually a String and not a string. And I think I'm fine with that even how it works now.

But what I actually want is to have unique Symbol or rather unique object in general. So we can narrow based on equality to singleton objects.

const singleton = {} as unique Record<string, any>;

const unknown = {a: "test"} as {a: string} | typeof singleton.

if (unknown != singleton) {
  unknown: {a: "test"};
  unknown.a; //test
}

If there's already a way to do this in the language, please let me know. If not, I'll open a feature request then.

@fatcerberus
Copy link

unique object sounds like nominal typing: see e.g. #202

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants