Skip to content

Avoid cast by providing type predicate to isExternalModuleAugmentation #22119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
3 commits merged into from
Mar 6, 2018

Conversation

ghost
Copy link

@ghost ghost commented Feb 22, 2018

This call to addIndirectUser turns out to be valid, but it took a while to figure that out -- can verify this with a type predicate.

@ghost ghost requested a review from armanio123 February 22, 2018 16:39
@ghost ghost force-pushed the isExternalModuleAugmentation branch from 25a1633 to e1aee28 Compare February 22, 2018 17:04
export function isAmbientModule(node: Node): boolean {
return node && node.kind === SyntaxKind.ModuleDeclaration &&
((<ModuleDeclaration>node).name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(<ModuleDeclaration>node));
export interface AmbientModuleDeclaration extends ModuleDeclaration { body?: ModuleBlock; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we move this to types?

}
}
else {
const state = declareModuleSymbol(node);
const { symbol } = node;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did this move?

@@ -1597,32 +1597,30 @@ namespace ts {
if (hasModifier(node, ModifierFlags.Export)) {
errorOnFirstToken(node, Diagnostics.export_modifier_cannot_be_applied_to_ambient_modules_and_module_augmentations_since_they_are_always_visible);
}
const { name } = node;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u can move this to the else block

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control flow was confused because node was already an AmbientModuleDeclaration and isExternalModuleAugmentation re-checks that (thus node was never in the else). Broke it into two functions to avoid that.

@ghost ghost merged commit 5e593ac into master Mar 6, 2018
@ghost ghost deleted the isExternalModuleAugmentation branch March 6, 2018 15:48
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant