Skip to content

Avoid cast by providing type predicate to isExternalModuleAugmentation#22119

Merged
3 commits merged into
masterfrom
isExternalModuleAugmentation
Mar 6, 2018
Merged

Avoid cast by providing type predicate to isExternalModuleAugmentation#22119
3 commits merged into
masterfrom
isExternalModuleAugmentation

Conversation

@ghost
Copy link
Copy Markdown

@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
Comment thread src/compiler/utilities.ts Outdated
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
Copy Markdown
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?

Comment thread src/compiler/binder.ts Outdated
}
else {
const state = declareModuleSymbol(node);
const { symbol } = node;
Copy link
Copy Markdown
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?

Comment thread src/compiler/binder.ts Outdated
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
Copy Markdown
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
Copy Markdown
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