moduleAugmentations may contain an Identifier#18009
Conversation
| function getTextOfLiteral(literal: LiteralExpression): string { | ||
| return literal.text; | ||
| function moduleNameIsEqualTo(a: StringLiteral | Identifier, b: StringLiteral | Identifier): boolean { | ||
| return a.kind === SyntaxKind.StringLiteral |
There was a problem hiding this comment.
Just to confirm, the string literal "global" is not equal to the identifier global?
There was a problem hiding this comment.
export {}; // this is a module
// Augments a module named "global"
declare module "global" {
export const x: number;
}
// Augments the global scope
declare global {
const y: number;
}
// Module access: "y" fails
import { x as _x, y as _y } from "global";
// Global access: "x" fails
x; y;| // file.imports may not be undefined if there exists dynamic import | ||
| let imports: StringLiteral[]; | ||
| let moduleAugmentations: StringLiteral[]; | ||
| let moduleAugmentations: Array<StringLiteral | Identifier>; |
There was a problem hiding this comment.
For my own edification, what's the difference between T[] and Array<T>?
There was a problem hiding this comment.
No semantic difference. I prefer to use Array<> when wrapping unions to avoid needing parentheses. (Array<A | B> instead of (A | B)[]). (See array-type (the "array-simple" option), which maybe we should start using.)
| // Because global augmentation doesn't have string literal name, we can check for global augmentation as such. | ||
| const nonGlobalAugmentation = filter(file.moduleAugmentations, (moduleAugmentation) => moduleAugmentation.kind === SyntaxKind.StringLiteral); | ||
| const moduleNames = map(concatenate(file.imports, nonGlobalAugmentation), getTextOfLiteral); | ||
| const moduleNames = getModuleImportAndAugmentationNames(file); |
There was a problem hiding this comment.
Should the name of this local indicate that it contains augmentations? (It may be implied - I don't know what an augmentation is.)
There was a problem hiding this comment.
A module augmentation adds new members to an external module. In order to do this we need to resolve that module, so we're considering both imports and augmentations together on purpose, since they both need to be resolved.
Handbook ref: http://www.typescriptlang.org/docs/handbook/declaration-merging.html
There was a problem hiding this comment.
It just seems strange that we say "ImportAndAugmentationNames" in the function name but just "moduleNames" in the local name.
There was a problem hiding this comment.
How about just calling the function getModuleNames then?
There was a problem hiding this comment.
If they're consistent, I'm happy. :)
| function getModuleImportAndAugmentationNames({ imports, moduleAugmentations }: SourceFile): string[] { | ||
| const res = imports.map(i => i.text); | ||
| for (const aug of moduleAugmentations) { | ||
| if (aug.kind === SyntaxKind.StringLiteral) { |
There was a problem hiding this comment.
What about the augmentations that are identifiers?
There was a problem hiding this comment.
We're interested in the names of modules that have to be resolved; declare global doesn't require any module resolution.
There was a problem hiding this comment.
Unless that's obvious to someone more experience, please consider adding a comment to that effect.
amcasey
left a comment
There was a problem hiding this comment.
Seems reasonable, but I don't have a deep understanding.
Fixes #17347