Fixed crash in checkExportsOnMergedDeclarationsWorker
#52740
Conversation
src/compiler/checker.ts
Outdated
| case SyntaxKind.MethodSignature: | ||
| case SyntaxKind.PropertySignature: | ||
| return getDeclarationSpaces(d.parent as any); |
There was a problem hiding this comment.
I don't really expect this to be the correct fix (at least not fully). I just pinpointed the place that causes it to crash and some preconditions for the crash.
I don't understand how DeclarationSpaces are used and what values those two should produce here. Note that at the moment we end up with DeclarationSpaces.ExportType in both added cases.
I also don't really understand the given test cases - I understand ESM and CJS but the TS syntaxes like export = _;, export as namespace _ (+ namespaces in general) always confuse me.
I just hope that a capable reviewer can suggest a proper fix 😉 perhaps @andrewbranch is a good person to ask about this stuff
There was a problem hiding this comment.
The parent of these kinds can only be TypeLiteralNode | InterfaceDeclaration which are definitely declarations in the type space. You can just add these cases to the top block of cases. I don’t think understanding the test input code is actually necessary or even helpful here 😆
There was a problem hiding this comment.
Ok, I changed this line to return DeclarationSpaces.ExportType;. Thanks for the tip! Do you think that this switch/case might also be missing some other member-like kinds from TypeLiteralNode | InterfaceDeclaration?
There was a problem hiding this comment.
I don’t know enough about this code path to make sense of what this function needs to handle. I think it’s fine to let it be bug-report-driven at this point.
fixes #32231