Declaration emit includes function properties #26499
Conversation
It does this by printing the type as an object literal type:
```ts
function f() { }
f.p = 1
```
Appears in a d.ts as
```ts
declare var f: {
(): void;
p: number;
}
```
It would also be possible to represent it as a namespace merge. I'm not
sure which is better.
```ts
declare function f(): void;
declare namespace f {
export var p: number;
}
```
In order to avoid a private-name-used error (though I think it was
actually *unused*), I also had to change the nodeBuilder code to match.
This is arguably harder to read. So it's possible that I should instead
keep the nodeBuilder version as `typeof f` and make an exception for
private name use.
|
@weswigham Any opinions on the right approach here? I was about to try the namespace merge, but I realised that I don't know how to make transformTopLevelDeclaration return two declarations. |
Returning two declarations in an array should be sufficient. A VisitResult is either a T, an undefined, or an array of T. |
| @@ -952,6 +952,11 @@ namespace ts { | |||
| } | |||
| case SyntaxKind.FunctionDeclaration: { | |||
| // Generators lose their generator-ness, excepting their return type | |||
| if (resolver.isJSContainerFunctionDeclaration(input)) { | |||
| const varDecl = createVariableDeclaration(input.name!, resolver.createTypeOfDeclaration(input, enclosingDeclaration, declarationEmitNodeBuilderFlags, symbolTracker), /*initializer*/ undefined); | |||
| const statement = createVariableStatement(needsDeclare ? [createModifier(SyntaxKind.DeclareKeyword)] : [], createVariableDeclarationList([varDecl], NodeFlags.Const)); | |||
There was a problem hiding this comment.
I'd use ensureModifiers(input) instead of the needsDeclare check here.
This makes the change smaller, overall.
Also improve emit style to match other namespace emit.
| const varDecl = createVariableDeclaration(unescapeLeadingUnderscores(p.escapedName), type, /*initializer*/ undefined); | ||
| return createVariableStatement(/*modifiers*/ undefined, createVariableDeclarationList([varDecl])); | ||
| }); | ||
| const namespaceDecl = createModuleDeclaration(/*decorators*/ undefined, ensureModifiers(input), input.name!, createModuleBlock(declarations), NodeFlags.Namespace); |
There was a problem hiding this comment.
Probably needs to be ensureModifiers(input, isPrivate) to inherit the scope's privacy correctly.
There was a problem hiding this comment.
Matters with a case like
namespace Foo {
function bar(): void {}
bar.num = 42;
export function foo() {
return bar;
}
}
|
@weswigham mind taking a look at the latest commit? I added your example, but the output looks wrong to me — I expected to see an object literal type, not |
|
Nope, it's perfect. The name shows up because it's visible internally at that location, which is all that matters. |
It does this by printing the type as an object literal type:
Appears in a d.ts as
It would also be possible to represent it as a namespace merge. I'm not sure which is better.
In order to avoid a private-name-used error (though I think it was actually unused), I also had to change the nodeBuilder code to match. This is arguably harder to read. So it's possible that I should instead keep the nodeBuilder version as
typeof fand make an exception for private name use.Fixes #26485