Skip to content

Commit 22cb2e6

Browse files
committed
Avoid the double-symbol trick for enums
Nameless jsdoc typedefs have their exportedness controlled by the exportedness of the location they pull their name from. Fixes microsoft#33575.
1 parent bae111f commit 22cb2e6

File tree

6 files changed

+93
-7
lines changed

6 files changed

+93
-7
lines changed

Diff for: src/compiler/binder.ts

+17-2
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ namespace ts {
541541
}
542542

543543
function declareModuleMember(node: Declaration, symbolFlags: SymbolFlags, symbolExcludes: SymbolFlags): Symbol {
544-
const hasExportModifier = getCombinedModifierFlags(node) & ModifierFlags.Export;
544+
const hasExportModifier = !!(getCombinedModifierFlags(node) & ModifierFlags.Export) || jsdocTreatAsExported(node);
545545
if (symbolFlags & SymbolFlags.Alias) {
546546
if (node.kind === SyntaxKind.ExportSpecifier || (node.kind === SyntaxKind.ImportEqualsDeclaration && hasExportModifier)) {
547547
return declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes);
@@ -567,7 +567,7 @@ namespace ts {
567567
// and this case is specially handled. Module augmentations should only be merged with original module definition
568568
// and should never be merged directly with other augmentation, and the latter case would be possible if automatic merge is allowed.
569569
if (isJSDocTypeAlias(node)) Debug.assert(isInJSFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
570-
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypeAlias(node)) {
570+
if (!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) {
571571
if (!container.locals || (hasSyntacticModifier(node, ModifierFlags.Default) && !getDeclarationName(node))) {
572572
return declareSymbol(container.symbol.exports!, container.symbol, node, symbolFlags, symbolExcludes); // No local symbol for an unnamed default!
573573
}
@@ -583,6 +583,21 @@ namespace ts {
583583
}
584584
}
585585

586+
function jsdocTreatAsExported(node: Node) {
587+
if (!isJSDocTypeAlias(node)) return false;
588+
// jsdoc typedef handling is a bit of a doozy, but to summarize, treat the typedef as exported if:
589+
// 1. It has an explicit name (since by default typedefs are always directly exported, either at the top level or in a container), or
590+
if (!isJSDocEnumTag(node) && !!node.fullName) return true;
591+
// 2. The thing a nameless typedef pulls its name from is implicitly a direct export (either by assignment or actual export flag).
592+
const declName = getNameOfDeclaration(node);
593+
if (!declName) return false;
594+
if (isPropertyAccessEntityNameExpression(declName.parent) && isTopLevelNamespaceAssignment(declName.parent)) return true;
595+
if (isDeclaration(declName.parent) && getCombinedModifierFlags(declName.parent) & ModifierFlags.Export) return true;
596+
// This could potentially be simplified by having `delayedBindJSDocTypedefTag` pass in an override for `hasExportModifier`, since it should
597+
// already have calculated and branched on most of this.
598+
return false;
599+
}
600+
586601
// All container nodes are kept on a linked list in declaration order. This list is used by
587602
// the getLocalNameOfContainer function in the type checker to validate that the local name
588603
// used for a container is unique.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
=== tests/cases/conformance/jsdoc/def.js ===
2+
/** @enum {number} */
3+
const MyEnum = {
4+
>MyEnum : Symbol(MyEnum, Decl(def.js, 1, 5), Decl(def.js, 0, 4))
5+
6+
a: 1,
7+
>a : Symbol(a, Decl(def.js, 1, 16))
8+
9+
b: 2
10+
>b : Symbol(b, Decl(def.js, 2, 7))
11+
12+
};
13+
export default MyEnum;
14+
>MyEnum : Symbol(MyEnum, Decl(def.js, 1, 5), Decl(def.js, 0, 4))
15+
16+
=== tests/cases/conformance/jsdoc/use.js ===
17+
import MyEnum from "./def";
18+
>MyEnum : Symbol(MyEnum, Decl(use.js, 0, 6))
19+
20+
/** @type {MyEnum} */
21+
const v = MyEnum.b;
22+
>v : Symbol(v, Decl(use.js, 3, 5))
23+
>MyEnum.b : Symbol(b, Decl(def.js, 2, 7))
24+
>MyEnum : Symbol(MyEnum, Decl(use.js, 0, 6))
25+
>b : Symbol(b, Decl(def.js, 2, 7))
26+
+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/conformance/jsdoc/def.js ===
2+
/** @enum {number} */
3+
const MyEnum = {
4+
>MyEnum : { a: number; b: number; }
5+
>{ a: 1, b: 2} : { a: number; b: number; }
6+
7+
a: 1,
8+
>a : number
9+
>1 : 1
10+
11+
b: 2
12+
>b : number
13+
>2 : 2
14+
15+
};
16+
export default MyEnum;
17+
>MyEnum : number
18+
19+
=== tests/cases/conformance/jsdoc/use.js ===
20+
import MyEnum from "./def";
21+
>MyEnum : { a: number; b: number; }
22+
23+
/** @type {MyEnum} */
24+
const v = MyEnum.b;
25+
>v : number
26+
>MyEnum.b : number
27+
>MyEnum : { a: number; b: number; }
28+
>b : number
29+

Diff for: tests/baselines/reference/jsEnumTagOnObjectFrozen.symbols

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ const Thing = Object.freeze({
5555
});
5656

5757
exports.Thing = Thing;
58-
>exports.Thing : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
59-
>exports : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
60-
>Thing : Symbol(Thing, Decl(index.js, 4, 3), Decl(index.js, 0, 4))
58+
>exports.Thing : Symbol(Thing, Decl(index.js, 4, 3))
59+
>exports : Symbol(Thing, Decl(index.js, 4, 3))
60+
>Thing : Symbol(Thing, Decl(index.js, 4, 3))
6161
>Thing : Symbol(Thing, Decl(index.js, 1, 5), Decl(index.js, 0, 4))
6262

6363
/**
+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
5+
// @Filename: def.js
6+
/** @enum {number} */
7+
const MyEnum = {
8+
a: 1,
9+
b: 2
10+
};
11+
export default MyEnum;
12+
13+
// @Filename: use.js
14+
import MyEnum from "./def";
15+
16+
/** @type {MyEnum} */
17+
const v = MyEnum.b;

Diff for: tests/cases/fourslash/quickInfoJSExport.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
//// export { test/**/String };
1616

1717
verify.quickInfoAt("",
18-
`type testString = string
19-
(alias) type testString = any
18+
`(alias) type testString = string
2019
(alias) const testString: {
2120
one: string;
2221
two: string;

0 commit comments

Comments
 (0)