Correctly print global augmentations#18660
Conversation
| write(node.flags & NodeFlags.Namespace ? "namespace " : "module "); | ||
| if (node.flags & NodeFlags.GlobalAugmentation) { | ||
| if (!hasModifier(node, ModifierFlags.Ambient)) { | ||
| // Always emit a 'declare' keyword in case it wasn't provided by a factory function call. |
There was a problem hiding this comment.
Maybe we should just throw an error when factory functions get invalid data? What's the protocol for something like this @rbuckton ?
There was a problem hiding this comment.
I think throwing an error is better, or changing the type to require the declare keyword.
There was a problem hiding this comment.
we do not do this in other places, why not just emit modifiers?
There was a problem hiding this comment.
declare isn't always legal, such as when meeting a global augmentation in a module augmentation:
// assuming there is a "foo" module...
declare module "foo" {
global {
}
}There was a problem hiding this comment.
That is a good point, and @rbuckton I didn't realize that was even legal.
sandersn
left a comment
There was a problem hiding this comment.
I agree with preventing construction of bad global augmentations in the first place.
| @@ -0,0 +1 @@ | |||
| declare global { } No newline at end of file | |||
There was a problem hiding this comment.
why does this end up in a .js? I expected a .d.ts or something.
There was a problem hiding this comment.
the printer does not care what it prints to. the transforms are the one that do.
| write(node.flags & NodeFlags.Namespace ? "namespace " : "module "); | ||
| if (node.flags & NodeFlags.GlobalAugmentation) { | ||
| if (!hasModifier(node, ModifierFlags.Ambient)) { | ||
| // Always emit a 'declare' keyword in case it wasn't provided by a factory function call. |
There was a problem hiding this comment.
I think throwing an error is better, or changing the type to require the declare keyword.
Apparently, they don't always need them!
Fixes #18657