Skip to content

Annotate immediately-invoked functions for inlining flow control analysis #11498

Description

Per #9998 and its many offshoots, we have problems analyzing code like this

let x: string | number = "OK";
mystery(() => {
  x = 10;
});
if (x === 10) { // Error, we believe this to be impossible  
}

The problem is that we don't know if mystery invokes its argument "now" or "later" (or possibly even never!).

The converse problem appears during reads:

let x: string | number = ...;
if (typeof x === 'string') {
  // Error, toLowerCase doesn't exist on string | number
  let arr = [1, 2, 3].map(n => x.toLowerCase());
}

Proposal is to allow some form of indicating that the function is invoked immediately, e.g.

declare function mystery(arg: immediate () => void): void;

This would "inline" the function body for the purposes of flow control analysis

Activity

  1. tinganho commented on Oct 11, 2016

    @tinganho
    Contributor

    I suggested something similar in #7770 (comment).

    Though after thinking about it, I thought it was better to annotate callbacks as non-immediate #10180 (comment):

    Immediate case:

    function doSomething(callback: () => any) {
         callback();
    }
    
    function fn(x: number|string) {
      if (typeof x === 'number') {
        doSomething(() => x.toFixed()); // No error.
      }
    }

    Non-immediate case:

    function doSomething(callback: async () => any) {
         setTimeout(callback, 0);
    }
    
    function fn(x: number|string) {
      if (typeof x === 'number') {
        doSomething(() => x.toFixed()); // Error x is  'number|string'
      }
    }

    Sync callbacks is only assignable to sync callbacks and vice versa for Async callbacks:

    function doSomething(callback: () => any) {
         setTimeout(callback, 0); // Error a sync callback is not assignable to an async one.
    }
    function doSomething1(callback: () => any) {
         callback();
    }
    function doSomething2(callback: () => any) {
         doSomething1(callback); // No error
    }

    Though re-using async keyword as a type annotation might conflict with the meaning of async declarations in async/await which makes a function returning a promise. An alternative is to use nonimmediate, though I think that keyword is too long.
    .

  2. yortus commented on Oct 11, 2016

    @yortus
    Contributor

    For reference, there was also some discussion of deferred and immediate modifiers in #9757.

  3. yortus commented on Oct 11, 2016

    @yortus
    Contributor

    Ryan Cavanaugh (@RyanCavanaugh) you commented here that implementing this kind of type modifier would require a massive architectural change. Is that still the case?

  4. kitsonk commented on Oct 11, 2016

    @kitsonk
    Contributor

    For completeness #11463 proposed an alternative solution for this problem. Since that was just an "escape hatch" and this is an instruction to CFA to better understand the code, I would prefer this.

    The use case where we have run into this is converting a Promise into a deferral and having to generate a noop function to get around strict null checks:

    const noop = () => { };
    
    function createDeferral() {
        let complete = noop;
        let cancel = noop;
        const promise = new Promise<void>((resolve, reject) => {
            complete = resolve;
            cancel = reject;
        });
    
        return { complete, cancel, promise };
    }
    
    const deferral = createDeferral();

    Though after thinking about it, I thought it was better to annotate callbacks as non-immediate

    While it is more "burden" on the developer, in the sense of "strictness" I still think it is safer to assume the worst (that it might not be immediate). Like with strictNullChecks overall, so I would be only in favour of an annotation that indicates that it is immediate.

  5. kitsonk commented on Oct 11, 2016

    @kitsonk
    Contributor

    Also 🚲 🏠 while it might be a tad bit confusing, why not sync to be a counter to async:

    interface PromiseConstructor {
        new <T>(executor: sync (resolve: (value?: T | PromiseLike<T>) => void, reject: (reason?: any) => void) => void): Promise<T>;
    }
  6. tinganho commented on Oct 11, 2016

    @tinganho
    Contributor

    I forgot to mention my points on why I think annotating it as async rather sync.

    1. Annotating it as immediate or sync means nearly all callbacks needs to be annotated, assuming most callbacks is sync. Async is the exception, sync is the rule.
    2. We already annotate async functions(as in async/await) as async. Sync functions has no annotations. So to keep consistent with the current design we should not annotate with immediate.

    Maybe to minimize confusion that async only accept a Promise returning callback, a deferred keyword might be more desirable.

  7. yortus commented on Oct 11, 2016

    @yortus
    Contributor

    I think to cover all cases of "used before assigned" checks you would need both a deferred and an immediate modifier.

    The deferred modifier would be needed in cases like Kitson Kelly (@kitsonk) reported here where a non-immediately invoked callback references a variable that is initialized later in the calling scope (i.e. lexically later but temporally earlier). immediate wouldn't help with that (unless the compiler assumes that an un-annotated callback must be deferred).

  8. yortus commented on Oct 11, 2016

    @yortus
    Contributor

    What about the interplay between immediate vs deferred invocation and synchronous vs asynchronous functions, which can occur in any combination?

    function invokeImmediately<R>(cb: () => R) {
        return cb();
    }
    
    let p = invokeImmediately(async () => {
        console.log(1);
        await null;
        console.log(2);
    });
    
    console.log(3);
    
    p.then(() => {
        console.log(4);
    });
    
    // Output:
    // 1
    // 3
    // 2
    // 4

    Here we have immediate invocation of an asynchronous function (fairly common in real code). As shown by the output, the async function executes synchronously until it either awaits or returns. Then the rest of the function is deferred to another tick.

    It would be pretty hard to do accurate CFA in this case with types and assignments involved, since some of the callback happens 'in line' with the surrounding code and some execution is deferred until later.

  9. novemberborn commented on Oct 11, 2016

    @novemberborn

    Tingan Ho (@tinganho),

    Annotating it as immediate or sync means nearly all callbacks needs to be annotated, assuming most callbacks is sync. Async is the exception, sync is the rule.

    In the new Promise() example, the executor function implements the Revealing Constructor Pattern. I wouldn't classify it as a callback — the constructor guarantees it's invoked immediately. Callbacks are often invoked in a future turn.

    Troy Gerwien (@yortus),

    Here we have immediate invocation of an asynchronous function (fairly common in real code). As shown by the output, the async function executes synchronously until it either awaits or returns. Then the rest of the function is deferred to another tick.

    It would be pretty hard to do accurate CFA in this case with types and assignments involved, since some of the callback happens 'in line' with the surrounding code and some execution is deferred until later.

    Not knowing anything about how CFA is actually implemented, presumably it could still make a prediction up until the first await statement?

    For both your points it would seem that sync doesn't quite cover the nuance of immediate invocation, so immediate may be a better keyword.

  10. tinganho commented on Oct 11, 2016

    @tinganho
    Contributor

    Mark Wubben (@novemberborn) I'm assuming though no annotation is immediate?

  11. novemberborn commented on Oct 11, 2016

    @novemberborn

    Tingan Ho (@tinganho),

    I'm assuming though no annotation is immediate?

    How do you mean?

  12. kitsonk commented on Oct 11, 2016

    @kitsonk
    Contributor

    Still think it would be dangerous to have implicit immediate. There are many callbacks where it doesn't matter and if it does matter, the developer should be explicit in order for CFA not to make assumptions.

  13. tinganho commented on Oct 11, 2016

    @tinganho
    Contributor

    Mark Wubben (@novemberborn)

    In the new Promise() example, the executor function implements the Revealing Constructor Pattern. I wouldn't classify it as a callback — the constructor guarantees it's invoked immediately. Callbacks are often invoked in a future turn

    In below, my arguments so far have been that no annotation means sync:

    interface PromiseConstructor {
        new <T>(executor: (resolve: (value?: T | PromiseLike<T>) => void /* is sync*/, reject: (reason?: any) => void) => void): Promise<T>;
    }
  14. 49 remaining items

  15. rChaoz commented on Nov 10, 2022

    @rChaoz

    I find it pretty weird that this issue still hasn't been tackled 6 years later. IMO, this fix of this issue has nothing to do with annotating lambdas as immediate/sync or async. I understand that it might work as a fix, but it feels more like a workaround. I don't think it touches the root of the problem: the compiler "assumes the best". I think it makes more sense for the compiler to "assume the worst", instead. ("assuming the best" means here that the compiler makes completely arbitrary assumptions in order to narrow the type as much as possible).

    let x: string | number | null = null;
    
    // Why would the compiler assume arbitrarily that the function is not called immediately?
    // It could or it could not be instantly invoked - so should assume the worst (i.e. both)
    instantInvoke(() => {x = "hello"});
    // at this point, the variable could either stay null, or it could be "hello", the type should be string | null
    if (x != null) console.log(x.length);

    The compiler is able to do this with an if statement with an unknown condition:
    image
    Why is the earlier case being treated differently, from a design point of view? I believe this is the real issue here.

    At the same time, I don't believe the second issue of widening the variable inferred type is actually an issue:

    let x: string | number =  ["hello", 5][0];
    if (typeof x === 'string') {
        let arr = instantInvoke(() => x.length);
    }

    Here, the compiler is assuming the worst, as it should: either the lambda is invoked immediately (it's a string, due to the if), or it's invoked later (variable could've been changed to a different value - any string | number). So, the inferred type should be string | (string | number), which simplifies to just string | number.

  16. SergeyPoznyakAkvelon commented on Jun 15, 2023

    @SergeyPoznyakAkvelon

    Workaround that worked for me:

    const _hack: { myMutableVariable: string | null } = {myMutableVariable: null}
    
    immediateInvoke(() => { _hack.myMytableVariable = 'hello' })
    
    _hack.myMutableVariable // type: string | null
  17. egucciar commented on Aug 9, 2023

    @egucciar

    +1 to Matei Trandafir (@rChaoz) recommendation to "assume the worst" , narrowing the type such that it won't let you write proper code seems like a weird bug in TS

  18. egucciar commented on Aug 9, 2023

    @egucciar

    Martin Johns (@MartinJohns) can you help me understand why this is a desirable experience?

    function testA() {
        let a: string | null = null;
        const list = [1];
        list.forEach((item) => {
            a = "hello";
        });
        if (a !== null) {
            a.trim(); // TS2339: Property 'trim' does not exist on type 'never'.
        }
    }
    

    Wouldn't it make more sense to preserve the type string | null since TS cannot assume the callback within the forEach did not run in between? Why should typescript be enforcing things that are not actually issues?

  19. egucciar commented on Aug 9, 2023

    @egucciar

    I read through some of the related comment threads and it appears as though this is not something TypeScript will fix due to it widening the types / removing narrowing hence breaking existing workflows. I saw some suggestions to add compilerOptions but this was rejected. I honestly rarely run into these issues with "real" code regardless and I would rather use a .reduce (or other array functions) than do a callback such as this, though its very surprising this is the existing behavior with no way to opt out, I'd rarely want to write my code this way to begin with.

  20. DmSor commented on Sep 21, 2023

    @DmSor

    Workaround that worked for me:

    let x = "OK" as string | number;
  21. fatcerberus commented on Nov 15, 2023

    @fatcerberus

    Matei Trandafir (@rChaoz) You're basically asking that TS be pessimistic in all cases - #9998 talks about this, but in summary: The compiler doesn't analyze the contents of called functions or passed callbacks for performance reasons (and sometimes doesn't even know what a called function does), and cancelling all narrowings every time any function is called would be impractical (people would just start putting type assertions everywhere) - hence the request for an immediate annotation to opt in to the more expensive analysis on a function-by-function basis.

    Anyway... #56407 involves an array.map() and this made me realize, being able to annotate as immediately-invoked isn't actually enough--we'd also need a way to annotate callbacks that should be analyzed as loop bodies.

  22. malthe commented on Feb 7, 2024

    @malthe

    For mystery and Promise the problem right now is that the compiler assumes deferred, refusing to widen the type. With an immediate typing, it could widen the type but as Aluan Haddad (@aluanhaddad) mentions, at the cost of a redundant check.

    Perhaps immediate always is an acceptable solution?

    • defer – this is the default behavior which gives 'item.data' is possibly 'undefined' in Ternary operator type daemon failed #56407 and incorrectly narrows the type in the mystery example.
    • immediate – this would fix 'item.data' is possibly 'undefined' and widen the type in the mystery example.
    • immediate always – this would fix the mystery example.

    Since defer represents the default semantics, there's no reason to have the keyword of course.

    Ryan Cavanaugh (@RyanCavanaugh) I assume that it's still the case that "reachability and control flow analysis is done before typechecking" as you mentioned in #9757 (comment). That probably means this is all pretty unrealistic, but even so it would be useful to agree on the ambition.

    Here's another example that illustrates why the current narrowing is bad:

    class Foo {
        private n?: number;
    
        async test() {
            this.n = 123;
            await new Promise(resolve => {
                this.n = undefined;
                setTimeout(resolve, 1000);
            });
            const n: number = this.n;
            console.log(n);
        }
    }

    The narrowing of this.n to just number after await is incorrect:

    1. The arrow function is called immediately.
    2. If the object is used concurrently, this.n could have been reassigned this way too.
  23. Igorbek commented on Jun 6, 2024

    @Igorbek
    Contributor

    Regarding the annotation using defer, immediate, immediate always, etc, I think that number of possible permutations to describe the possible CFA states would be enormous and even if these seem to solve some of the common case, it still would be limited in what you can possibly describe.

    As a discussion, I would suggest considering some form of richer assertions about the function intent as part of their signature. My idea is to have some asserts block in the return type position which can contain some limited code that would make some sense to CFA to infer possible states at the call site.

    declare function mystery1(cb: () => void): void & asserts {
      cb() // same as 'immediate always', but also says it is invoked exactly once
    }
    
    declare function mystery2(cb: () => void): void & asserts {
      if (_) {
        cb() // invoked optionally, but immediately (sync)
      }
    }
    
    declare function mystery3(cond: boolean, cb: () => void): void & asserts {
      if (cond) { // the condition can be used in CFA too
        cb() // invoked optionally based on provided condition, but immediately (sync) and once only
      }
    }
    
    mystery3(true, () => { ... }) // CFA knows it is called
    mystery3(false, () => { ... }) // CFA knows it is not called
    
    declare function mystery4(cb: () => void): Promise<void> & asserts {
      await _
      cb() // is called asynchronously, same as 'defer'
    }

    The allowed code in the asserts { ... } block should be very limited subset of TS with clear patterns (the subset of what CFA can make sense of) what it would mean:

    • invoking of the callbacks, based on known or unknown (_ as an example of an unknown sigil) conditions
    • using await to indicate that some are deferred
    • number of calls
      • cb(); cb(); would say it is invoked twice
      • while (_) { cb() } would say it is possibly invoked multiple times (0+)
    • order of invoking: a(); b(); would imply b is called after a
  24. dead-claudia commented on Jun 7, 2024

    @dead-claudia

    Igor Oleinikov (@Igorbek) Callback timing doesn't suffer from that much combinatorial explosion - there's only four possible combinations (the comment you're replying to misses one of them). And everything else (is, asserts, etc.) is orthogonal.

    Think of it this way:

    • A function can only either 1. be potentially called after return or 2. not potentially be called after return. The presence of immediate indicates 2, and the absence of it indicates 1.
    • A function can only either 1. be always called some point before return or 2. potentially not be called before return. The presence of always indicates 1, and the absence of it indicates 2.

    These two aren't mutually exclusive on the surface:

    • func in new Promise(func) should have both immediate and always. It's always called synchronously.
    • func in array.filter(func) should have immediate but not always. The array may be empty, but is never called async.
    • func in queueMicrotask(func) should (in browsers) have always but not immediate in its type. It's always scheduled to run async, assuming the event loop isn't shut down. (If/when it becomes cancellable, that may change.)
    • func in setTimeout(func, ms) should have neither always nor immediate in its type. The timer could get cancelled.

    For control flow checking, the last two have the same effect. Async doesn't imply timing at all, and so there's no use in allowing always without immediate. immediate without always is not unlike initializing/assigning inside an if, so it could still impact flow-sensitive typing.

  25. nicosampler commented on Aug 27, 2024

    @nicosampler

    Workaround that worked for me:

    let x = "OK" as string | number;

    I would give you a hug if I could

  26. AnthonyLenglet commented on Dec 12, 2024

    @AnthonyLenglet

    just stumbled into this issue as well

    async function dbCall(fn: (client: MongoClient) => Promise<void>) {
      let client: MongoClient | undefined = undefined;
      try {
        if (!process.env.MONGODB_URL) {
          throw new Error("MONGODB_URL is not defined");
        }
    
        client = new MongoClient(process.env.MONGODB_URL);
    
        await fn(client);
      } catch (err) {
        console.error(err);
      } finally {
        await client?.close();
      }
    }
    async function getActivity(_id: ObjectId) {
      let activity: IBaseActivity | null;
    
      await dbCall(async (client) => {
        activity = await client
          .db("bi")
          .collection("activities")
          .findOne<IBaseActivity>({ _id });
      });
    
      return activity ? new BaseActivity(activity) : null; // <- Variable 'activityPole' is used before being assigned.
    }

    would love to see something like this get implemented, definitely a new pain point for me

  27. mfplunet commented on Feb 5, 2025

    @mfplunet

    Hi, this massive bikeshedding is good to find a proper solution but I think in the meantime we should fix the incorrect error in cases like #61125.

  28. rChaoz commented on Mar 4, 2026

    @rChaoz

    Related - similar issue solved by Kotlin using contracts.

    Kotlin uses contracts to indicate conditions - for a example a function that throws if arg is null might have:

    fun check(arg: String?) {
        contract {
            returns() implies (arg != null)
        }
        if (arg == null) throw ...
    }

    The compiler then narrows, for the caller, the argument variable type, from String? to String. This is similar to TypeScript type guard functions.

    Where it gets interesting is that it also defines what functions do with lambdas:

    fun instantInvoke(lambda: () -> Unit) {
        contract {
            callsInPlace(lambda, InvocationKind.EXACTLY_ONCE)
        }
        lambda()
    }

    This allows the compiler to infer stuff like:

    var x: Int?
    instantInvoke { x = 10 }
    // compiler knows x is definitely defined here
    print(x + 2)

    Docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Needs ProposalThis issue needs a plan that clarifies the finer details of how it could be implemented.SuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions