Improve errors for incorrectly nested export default #43967
Conversation
The compiler and services don't handle incorrectly nested
`export default` well right now:
```ts
export = (x,y) => {
export default { }
}
```
Asking for document highlights, find all references or quick info on
'export' or 'default' cause a crash. After the crash is fixed, the error
message is confusing and wrong: "An export assignment cannot be used outside a module."
This PR:
1. Skips document highlights for incorrectly nested export default.
2. Skips find all refs for incorrectly nested export default.
3. Switches the fallback binding for incorrectly nested export default
from Alias to Property. Neither is correct, but Property doesn't cause a
crash in alias resolution.
4. Improves the error message to reflect a post-ES module world, which
has export default and 'module' means 'ES module', not 'namespace'.
Fixes #40082 and the related bugs mentioned above.
typescript-bot
added
the
For Milestone Bug
| @@ -465,13 +465,13 @@ namespace ts.FindAllReferences { | |||
|
|
|||
| function getExport(): ExportedSymbol | ImportedSymbol | undefined { | |||
| const { parent } = node; | |||
| const grandParent = parent.parent; | |||
| const grandparent = parent.parent; | |||
There was a problem hiding this comment.
grandparent is one word, not "grand parent". That's just insulting.
src/services/services.ts
Outdated
| @@ -1725,7 +1725,7 @@ namespace ts { | |||
|
|
|||
| function getDocumentHighlights(fileName: string, position: number, filesToSearch: readonly string[]): DocumentHighlights[] | undefined { | |||
| const normalizedFileName = normalizePath(fileName); | |||
| Debug.assert(filesToSearch.some(f => normalizePath(f) === normalizedFileName)); | |||
| Debug.assert(filesToSearch.some(f => normalizePath(f) === normalizedFileName), `Could not find ${normalizedFileName} in ${filesToSearch}`); | |||
There was a problem hiding this comment.
Uh, could this result in leaking filenames into telemetry? I might need to remove this.
There was a problem hiding this comment.
Paths are generally okay, though we've had problems with things like filesToSearch being too long and causing volume problems.
There was a problem hiding this comment.
Wait really? I thought paths were a hard no. Many, if not most, will have usernames in them.
There was a problem hiding this comment.
I'll remove it, although I noticed that @amcasey added a throw of another path in getValidSourceFile a couple of years ago.
| @@ -174,7 +174,7 @@ namespace ts.SymbolDisplay { | |||
| } | |||
|
|
|||
| let signature: Signature | undefined; | |||
| type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol || symbol, location); | |||
There was a problem hiding this comment.
The first line of getTypeOfSymbolAtLocation does symbol = symbol.exportSymbol || symbol, so we don't need to do it here.
| @@ -2768,8 +2768,8 @@ namespace ts { | |||
|
|
|||
| function bindExportAssignment(node: ExportAssignment) { | |||
| if (!container.symbol || !container.symbol.exports) { | |||
| // Export assignment in some sort of block construct | |||
| bindAnonymousDeclaration(node, SymbolFlags.Alias, getDeclarationName(node)!); | |||
There was a problem hiding this comment.
in hindsight, creating an alias for an incorrect declaration is extremely likely to fail later.
| @@ -38301,7 +38301,9 @@ namespace ts { | |||
| } | |||
|
|
|||
| function checkExportAssignment(node: ExportAssignment) { | |||
| if (checkGrammarModuleElementContext(node, Diagnostics.An_export_assignment_can_only_be_used_in_a_module)) { | |||
| if (checkGrammarModuleElementContext(node, node.isExportEquals | |||
There was a problem hiding this comment.
This is weird because node.isExportEquals looks like the second argument at a glance.
The compiler and services don't handle incorrectly nested
export defaultwell right now:Asking for document highlights, find all references or quick info on 'export' or 'default' cause a crash. After the crash is fixed, the error message is confusing and wrong: "An export assignment cannot be used outside a module."
This PR:
crash in alias resolution.
Fixes #40082 and the related bugs mentioned above.