Code completion crashes when localStorage, postcss-object-fit-images, or gulp-watch packages are imported

[zfeher]

TypeScript Version: 3.2.0-dev.20181011
TypeScript Version: 3.1.3

Search Terms: code completion, auto import, intellisense

Code
You can check out this repo to reproduce the issue: repo link
If any of the following dependencies are imported in the project the code completion feature crashes: localStorage, postcss-object-fit-images or gulp-watch. Without them the feature works just fine.

Expected behavior: code completion works when the mentioned dependencies are imported

Actual behavior: code completion crashes if any of the following dependencies are imported in the project: localStorage, postcss-object-fit-images or gulp-watch

Related Issues: #27640

[ghost]

Repro:

/// <reference path="fourslash.ts" />

// @allowJs: true
// @checkJs: true

// @Filename: /a.js
////function f() {}
////if (something)
////    module.exports = f;
////else
////    module.exports = 42;

// @Filename: /b.js
////export const foo = 0;

// @Filename: /c.js
////foo

goTo.file("/c.js");
verify.importFixAtPosition([
`???`,
]);

There's no error if I remove if (something) and the else and just module.exports = f; at the top-level.
Testing without the if, I noticed that checker.resolveExternalModuleSymbol resolves directly to function f() without creating an alias symbol.
But with the if, it resolves to an alias symbol; so we go inside the if (defaultExport.flags & SymbolFlags.Alias) { in getDefaultExportInfoWorker. getImmediateAliasedSymbol(defaultExport) gives us function f() (which is correct), but then it has no parent for some reason -- which should be the source file symbol, right?

[DanielRosenwasser]

@sandersn can you verify whether this fixes #28133?

[sandersn]

No, it does not fix it. Sounds similar, but there's no module.exports merge.

And the crash doesn't go down the wrong branch of getTypeOfSymbol, although that's not necessarily indicative.

[sandersn]

One important trigger of the problem is that the two export assignments differ: one is an alias and the other is a value. The resulting symbol merges, unlike in TS, ending up with the flags Alias | Property | ExportValue | ValueModule. Now I need to track down where this makes the alias creation go wrong.

[sandersn]

An alias with Value flags blocks alias resolution, which is why checker.resolveExternalModuleSymbol incorrectly returns an alias (which getDefaultExportInfoWorker tries to resolve itself, but fails).

The fix might be to allow only all-Alias or all-Value module-export assignments. I'll have to see whether this causes a bunch of real-world failures though.

[sandersn]

Never mind; resolveSymbol has a special-case explicitly for assignment declarations. I just need to use SymbolFlags.Assignment | SymbolFlags.Alias when binding module.exports = *EntityName*.

[zfeher]

@sandersn I've just checked the issue in our full project too with the current build: [email protected] and the auto complete seems to be working \o/

Thank you for fixing it.