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

type narrowing for instance fields should be discarded after await #31429

Closed
AnyhowStep opened this issue May 16, 2019 · 6 comments
Closed

type narrowing for instance fields should be discarded after await #31429

AnyhowStep opened this issue May 16, 2019 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@AnyhowStep
Copy link
Contributor

AnyhowStep commented May 16, 2019

TypeScript Version: 3.4.1, 3.3.0

Search Terms: type narrow, instance field, discard, await

Code

class C {
    private b: boolean = false;
    async foo() {
        this.b = true;
        await new Promise(resolve => setTimeout(resolve, 1000));
        /*
            This condition will always return 'false' since
            the types 'true' and 'false' have no overlap.
            -----
            (property) C.b: true
        */
        if (this.b == false) {
            //After 1s, this will be logged
            console.log("b was set to false");
        }
    }

    setBToFalse() {
        this.b = false;
    }
}

const c = new C();
//This sets b to true, waits 1s, checks b
c.foo();
//We set b to false immediately
c.setBToFalse();

Expected behavior:

Comparison to false should work.

Type of this.b should be boolean.

Actual behavior:

Comparison to false does not work.

Type of this.b is true.

This example is contrived but my actual use case is with enum type comparisons.
I know the enum value can possibly change after the await but TS doesn't know that.

Workaround:

Use a type assertion,

if ((this.b as boolean) == false) {

}

Playground Link: Here

@nmain
Copy link

nmain commented May 16, 2019

Duplicate of #27900

@RyanCavanaugh
Copy link
Member

Duplicate #9998

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label May 16, 2019
@AnyhowStep
Copy link
Contributor Author

I'll close this duplicate but I guess I'll elaborate that my particular use case is for a class that acts as an asynchronous background processor.

So, while it's performing some tasks, certain state might change from under its feet. For example, at the beginning of a task, the "state" could be "RUNNING". And, in the middle, the user could ask the processor to stop. So, the "state" becomes "STOPPING".

In its loop, the processor checks the "state" to see if it is still "RUNNING", and continues if it is.

However, the flow analysis says it is always "RUNNING", which isn't true, in this case.

@fatcerberus
Copy link

fatcerberus commented May 16, 2019

In spite of #9998 (while I generally agree with the conclusions there) I think it might be worth consideration, at least, that await is fundamentally different from a normal function call; anything could happen before control of the async function resumes (and what happens in between is even more unpredictable than a synchronous call because we’re completely at the mercy of the event loop!).

Almost by definition, a function resuming from an await means that some state somewhere has changed, else the promise wouldn’t have resolved. This is opposed to a synchronous call, where assuming everything is pure is (usually) reasonable, I generally would never assume that the same is true of an asynchronous operation.

We already reset narrowings inside of function expressions, which manifests today for .then() calls. Maybe we should treat await promise the same way as we do promise.then(...)?

@RyanCavanaugh
Copy link
Member

Almost by definition, a function resuming from an await means that some state somewhere has changed, else the promise wouldn’t have resolve

This is hard for me to agree with, especially with the increasing prevalence of async functions and calls. For example:

async function fn(obj: { fileName?: string }) {
  if (obj.fileName !== undefined) {
    const contents = await fs.readFile(obj.fileName);
    console.log(obj.fileName.length); // Error...?
  }
}

We already reset narrowings inside of function expressions

The closer analog to awaited calls is IIFEs, which do inherit their surrounding narrowings.

@fatcerberus
Copy link

Good point re: prevalence of async calls, although there is still some nondeterminism introduced by the event loop (you don’t know how many or even which other event handlers will fire before your function gets to resume executing) that simply isn’t a factor with synchronous calls—though I suppose await exists precisely for the purpose of abstracting that away in the first place. Hmm. Tough call.

IIFEs, which do inherit their surrounding narrowings.

This I wasn’t aware of, thanks—I had thought narrowings were always reset at function boundaries and wasn’t aware of this exception. That said, await is syntactic sugar for .then(...) so from an end-user point of view I would have to agree it’s surprising for narrowings to be preserved in the await case but not the .then() case as they are semantically equivalent... but in the end whatever is done is necessarily going to be a compromise so... yeah. It’s interesting to think about at least, but whatever else can be said here has likely already been said in #9998. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants