Internal caching of Iterable types skips type checks for optional Arrays, only emits error on first encountered instance of TS error 2488

[cewkrupa]

🔎 Search Terms

compiler cache, 2488, must have a 'Symbol.iterator' method that returns an iterator, only error on first time, cached errors, first instance of error, checker

🕗 Version & Regression Information

Tested with the Typescript playground, with the listed versions in the dropdown.

• This changed between versions 4.0.5 and 4.1.5 (as tested on the playground), and is present in all later versions, including 5.8.3 and Nightly.
• While testing I also noticed that 3.5.1 has the expected behavior of reporting all instances of the error, but with a different error message. The errors go away entirely from 3.6.3 — 4.0.5.

⏯ Playground Link

https://www.typescriptlang.org/play/?target=99&moduleResolution=99&module=199&declaration=false&strictBuiltinIteratorReturn=true&useUnknownInCatchVariables=true&resolvePackageJsonExports=true&resolvePackageJsonImports=true&noLib=true#code/C4TwDgpgBA8mwEsD2A7AhgGwIICcdpCgF4oBXFAEwgDMEUIKoAfKAZ2BzoHMBtAXQDcAKCHVyAY0SooXCMFw4AFAEoAXLHjJ02PASgBvIVCg45pHCig8ARKgjWANFGvAA7kmt8hAXxHjU7FDUSEjEMnIKKsJCAPQxQWgIGKxQrgjAABZQACrg0ADkcFLaCgT5UAC2pIEZaABu0GhQ+TwAyiAVAEZIGAB06RD4wEg4fCrlFXIZSIyZaMAmZhYpaJYDQyO9igBMACwAHPvKQv4ogZ1oOGE8vbfBSIKx8azTpBgUKPkLmQgpdglJKDDJAAfhBJwCCwuAC9rrdevdBEA

💻 Code

type OptionalArray = undefined | string[];

function getArr(): OptionalArray {
  return ["one", "two"]
}

const foo = getArr();

// fails with Type 'OptionalArray' must have a '[Symbol.iterator]()' method that returns an iterator.(2488)
const bar = [...foo];
// shouldn't this one fail too??
const baz = [...foo];

🙁 Actual behavior

• Spreading a type that could be undefined or Array correctly emits an error the first time the issue is encountered in a single compilation.
• The second time the issue is encountered, no error is emitted.
• From debugging the Typescript compiler, I believe this is the problematic line, in getIterationTypesOfIterable.
  • The first time <undefined | Array> is encountered in the codebase, the iterable type is determined to be noIterationTypes. An error is emitted on line 45082, and then the type is cached for <undefined | Array> on line 45087.
  • The next time <undefined | Array> is encountered, the cached value of noIterationTypes is retrieved on line 45073, and getIterationTypesOfIterable returns undefined on the next line and does not report a type error.

Since checker.ts is too big for direct links to work, here’s the relevant function copied out.

/**
     * Gets the *yield*, *return*, and *next* types from an `Iterable`-like or `AsyncIterable`-like type.
     *
     * At every level that involves analyzing return types of signatures, we union the return types of all the signatures.
     *
     * Another thing to note is that at any step of this process, we could run into a dead end,
     * meaning either the property is missing, or we run into the anyType. If either of these things
     * happens, we return `undefined` to signal that we could not find the iteration type. If a property
     * is missing, and the previous step did not result in `any`, then we also give an error if the
     * caller requested it. Then the caller can decide what to do in the case where there is no iterated
     * type.
     *
     * For a **for-of** statement, `yield*` (in a normal generator), spread, array
     * destructuring, or normal generator we will only ever look for a `[Symbol.iterator]()`
     * method.
     *
     * For an async generator we will only ever look at the `[Symbol.asyncIterator]()` method.
     *
     * For a **for-await-of** statement or a `yield*` in an async generator we will look for
     * the `[Symbol.asyncIterator]()` method first, and then the `[Symbol.iterator]()` method.
     */
    function getIterationTypesOfIterable(type: Type, use: IterationUse, errorNode: Node | undefined) {
        if (isTypeAny(type)) {
            return anyIterationTypes;
        }

        if (!(type.flags & TypeFlags.Union)) {
            const errorOutputContainer: ErrorOutputContainer | undefined = errorNode ? { errors: undefined, skipLogging: true } : undefined;
            const iterationTypes = getIterationTypesOfIterableWorker(type, use, errorNode, errorOutputContainer);
            if (iterationTypes === noIterationTypes) {
                if (errorNode) {
                    const rootDiag = reportTypeNotIterableError(errorNode, type, !!(use & IterationUse.AllowsAsyncIterablesFlag));
                    if (errorOutputContainer?.errors) {
                        addRelatedInfo(rootDiag, ...errorOutputContainer.errors);
                    }
                }
                return undefined;
            }
            else if (errorOutputContainer?.errors?.length) {
                for (const diag of errorOutputContainer.errors) {
                    diagnostics.add(diag);
                }
            }
            return iterationTypes;
        }

        const cacheKey = use & IterationUse.AllowsAsyncIterablesFlag ? "iterationTypesOfAsyncIterable" : "iterationTypesOfIterable";
        const cachedTypes = getCachedIterationTypes(type, cacheKey);
        if (cachedTypes) return cachedTypes === noIterationTypes ? undefined : cachedTypes;

        let allIterationTypes: IterationTypes[] | undefined;
        for (const constituent of (type as UnionType).types) {
            const errorOutputContainer: ErrorOutputContainer | undefined = errorNode ? { errors: undefined } : undefined;
            const iterationTypes = getIterationTypesOfIterableWorker(constituent, use, errorNode, errorOutputContainer);
            if (iterationTypes === noIterationTypes) {
                if (errorNode) {
                    const rootDiag = reportTypeNotIterableError(errorNode, type, !!(use & IterationUse.AllowsAsyncIterablesFlag));
                    if (errorOutputContainer?.errors) {
                        addRelatedInfo(rootDiag, ...errorOutputContainer.errors);
                    }
                }
                setCachedIterationTypes(type, cacheKey, noIterationTypes);
                return undefined;
            }
            else if (errorOutputContainer?.errors?.length) {
                for (const diag of errorOutputContainer.errors) {
                    diagnostics.add(diag);
                }
            }

            allIterationTypes = append(allIterationTypes, iterationTypes);
        }

        const iterationTypes = allIterationTypes ? combineIterationTypes(allIterationTypes) : noIterationTypes;
        setCachedIterationTypes(type, cacheKey, iterationTypes);
        return iterationTypes === noIterationTypes ? undefined : iterationTypes;
    }

🙂 Expected behavior

Expected Behavior

• The Typescript compiler should report all instances of TS error 2488, not just the first one encountered.

Additional information about the issue

This bug presented itself while migrating a large React Typescript codebase to strict mode. The codebase was previously migrated to Typescript from untyped Javascript without strict mode enabled. Our migration plan involved turning strict mode on for the entire codebase, ignoring all existing strict mode errors, and then incrementally migrating existing errors while disallowing new strict mode errors.

Because we happened to ignore the very first instance of this error due to our migration plan, we never knew it was a problem in other files we type checked. The errors in other locations would only appear locally when developers would run in --watch mode and made a change to the files that were “skipped”, but never when type checking the project as a whole.