Skip to content

Commit eac0738

Browse files
sandersnsandersn
authored
Fix serialisation of static class members in JS (#37780)
* Fix serialisation of static class members in JS Previously static class members would be treated the same way as expando namespace assignments to a class: ```ts class C { static get x() { return 1 } } C.y = 12 ``` This PR adds a syntactic check to the static/namespace filter that treats symbols whose valueDeclaration.parent is a class as statics. Fixes #37289 * fix messed-up indent * Extract function
1 parent 20ecbb0 commit eac0738

File tree

5 files changed

+223
-13
lines changed

5 files changed

+223
-13
lines changed

‎src/compiler/checker.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5971,7 +5971,7 @@ namespace ts {
59715971
}
59725972

59735973
function getNamespaceMembersForSerialization(symbol: Symbol) {
5974-
return !symbol.exports ? [] : filter(arrayFrom((symbol.exports).values()), p => !((p.flags & SymbolFlags.Prototype) || (p.escapedName === "prototype")));
5974+
return !symbol.exports ? [] : filter(arrayFrom(symbol.exports.values()), isNamespaceMember);
59755975
}
59765976

59775977
function isTypeOnlyNamespace(symbol: Symbol) {
@@ -6103,7 +6103,7 @@ namespace ts {
61036103
}
61046104
// Module symbol emit will take care of module-y members, provided it has exports
61056105
if (!(symbol.flags & (SymbolFlags.ValueModule | SymbolFlags.NamespaceModule) && !!symbol.exports && !!symbol.exports.size)) {
6106-
const props = filter(getPropertiesOfType(type), p => !((p.flags & SymbolFlags.Prototype) || (p.escapedName === "prototype")));
6106+
const props = filter(getPropertiesOfType(type), isNamespaceMember);
61076107
serializeAsNamespaceDeclaration(props, localName, modifierFlags, /*suppressNewPrivateContext*/ true);
61086108
}
61096109
}
@@ -6159,6 +6159,10 @@ namespace ts {
61596159
}
61606160
}
61616161

6162+
function isNamespaceMember(p: Symbol) {
6163+
return !(p.flags & SymbolFlags.Prototype || p.escapedName === "prototype" || p.valueDeclaration && isClassLike(p.valueDeclaration.parent));
6164+
}
6165+
61626166
function serializeAsClass(symbol: Symbol, localName: string, modifierFlags: ModifierFlags) {
61636167
const localParams = getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol);
61646168
const typeParamDecls = map(localParams, p => typeParameterToDeclaration(p, context));
@@ -6195,12 +6199,9 @@ namespace ts {
61956199
emptyArray;
61966200
const publicProperties = flatMap<Symbol, ClassElement>(publicSymbolProps, p => serializePropertySymbolForClass(p, /*isStatic*/ false, baseTypes[0]));
61976201
// Consider static members empty if symbol also has function or module meaning - function namespacey emit will handle statics
6198-
const staticMembers = symbol.flags & (SymbolFlags.Function | SymbolFlags.ValueModule)
6199-
? []
6200-
: flatMap(filter(
6201-
getPropertiesOfType(staticType),
6202-
p => !(p.flags & SymbolFlags.Prototype) && p.escapedName !== "prototype"
6203-
), p => serializePropertySymbolForClass(p, /*isStatic*/ true, staticBaseType));
6202+
const staticMembers = flatMap(
6203+
filter(getPropertiesOfType(staticType), p => !(p.flags & SymbolFlags.Prototype) && p.escapedName !== "prototype" && !isNamespaceMember(p)),
6204+
p => serializePropertySymbolForClass(p, /*isStatic*/ true, staticBaseType));
62046205
const constructors = serializeSignatures(SignatureKind.Construct, staticType, baseTypes[0], SyntaxKind.Constructor) as ConstructorDeclaration[];
62056206
for (const c of constructors) {
62066207
// A constructor's return type and type parameters are supposed to be controlled by the enclosing class declaration
@@ -6363,7 +6364,7 @@ namespace ts {
63636364
includePrivateSymbol(referenced || target);
63646365
}
63656366

6366-
// We disable the context's symbol traker for the duration of this name serialization
6367+
// We disable the context's symbol tracker for the duration of this name serialization
63676368
// as, by virtue of being here, the name is required to print something, and we don't want to
63686369
// issue a visibility error on it. Only anonymous classes that an alias points at _would_ issue
63696370
// a visibility error here (as they're not visible within any scope), but we want to hoist them
@@ -6479,10 +6480,11 @@ namespace ts {
64796480
// need to be merged namespace members
64806481
return [];
64816482
}
6482-
if (p.flags & SymbolFlags.Prototype || (baseType && getPropertyOfType(baseType, p.escapedName)
6483-
&& isReadonlySymbol(getPropertyOfType(baseType, p.escapedName)!) === isReadonlySymbol(p)
6484-
&& (p.flags & SymbolFlags.Optional) === (getPropertyOfType(baseType, p.escapedName)!.flags & SymbolFlags.Optional)
6485-
&& isTypeIdenticalTo(getTypeOfSymbol(p), getTypeOfPropertyOfType(baseType, p.escapedName)!))) {
6483+
if (p.flags & SymbolFlags.Prototype ||
6484+
(baseType && getPropertyOfType(baseType, p.escapedName)
6485+
&& isReadonlySymbol(getPropertyOfType(baseType, p.escapedName)!) === isReadonlySymbol(p)
6486+
&& (p.flags & SymbolFlags.Optional) === (getPropertyOfType(baseType, p.escapedName)!.flags & SymbolFlags.Optional)
6487+
&& isTypeIdenticalTo(getTypeOfSymbol(p), getTypeOfPropertyOfType(baseType, p.escapedName)!))) {
64866488
return [];
64876489
}
64886490
const flag = (modifierFlags & ~ModifierFlags.Async) | (isStatic ? ModifierFlags.Static : 0);

‎tests/baselines/reference/jsDeclarationsClassStatic.js

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
//// [source.js]
2+
class Handler {
3+
static get OPTIONS() {
4+
return 1;
5+
}
6+
7+
process() {
8+
}
9+
}
10+
Handler.statische = function() { }
11+
const Strings = {
12+
a: "A",
13+
b: "B"
14+
}
15+
16+
module.exports = Handler;
17+
module.exports.Strings = Strings
18+
19+
/**
20+
* @typedef {Object} HandlerOptions
21+
* @property {String} name
22+
* Should be able to export a type alias at the same time.
23+
*/
24+
25+
26+
//// [source.js]
27+
var Handler = /** @class */ (function () {
28+
function Handler() {
29+
}
30+
Object.defineProperty(Handler, "OPTIONS", {
31+
get: function () {
32+
return 1;
33+
},
34+
enumerable: false,
35+
configurable: true
36+
});
37+
Handler.prototype.process = function () {
38+
};
39+
return Handler;
40+
}());
41+
Handler.statische = function () { };
42+
var Strings = {
43+
a: "A",
44+
b: "B"
45+
};
46+
module.exports = Handler;
47+
module.exports.Strings = Strings;
48+
/**
49+
* @typedef {Object} HandlerOptions
50+
* @property {String} name
51+
* Should be able to export a type alias at the same time.
52+
*/
53+
54+
55+
//// [source.d.ts]
56+
export = Handler;
57+
declare class Handler {
58+
static get OPTIONS(): number;
59+
process(): void;
60+
}
61+
declare namespace Handler {
62+
export { statische, Strings, HandlerOptions };
63+
}
64+
declare function statische(): void;
65+
declare namespace Strings {
66+
export const a: string;
67+
export const b: string;
68+
}
69+
type HandlerOptions = {
70+
/**
71+
* Should be able to export a type alias at the same time.
72+
*/
73+
name: String;
74+
};

‎tests/baselines/reference/jsDeclarationsClassStatic.symbols

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
=== tests/cases/conformance/jsdoc/declarations/source.js ===
2+
class Handler {
3+
>Handler : Symbol(Handler, Decl(source.js, 0, 0), Decl(source.js, 7, 1))
4+
5+
static get OPTIONS() {
6+
>OPTIONS : Symbol(Handler.OPTIONS, Decl(source.js, 0, 15))
7+
8+
return 1;
9+
}
10+
11+
process() {
12+
>process : Symbol(Handler.process, Decl(source.js, 3, 2))
13+
}
14+
}
15+
Handler.statische = function() { }
16+
>Handler.statische : Symbol(Handler.statische, Decl(source.js, 7, 1))
17+
>Handler : Symbol(Handler, Decl(source.js, 0, 0), Decl(source.js, 7, 1))
18+
>statische : Symbol(Handler.statische, Decl(source.js, 7, 1))
19+
20+
const Strings = {
21+
>Strings : Symbol(Strings, Decl(source.js, 9, 5))
22+
23+
a: "A",
24+
>a : Symbol(a, Decl(source.js, 9, 17))
25+
26+
b: "B"
27+
>b : Symbol(b, Decl(source.js, 10, 11))
28+
}
29+
30+
module.exports = Handler;
31+
>module.exports : Symbol("tests/cases/conformance/jsdoc/declarations/source", Decl(source.js, 0, 0))
32+
>module : Symbol(export=, Decl(source.js, 12, 1))
33+
>exports : Symbol(export=, Decl(source.js, 12, 1))
34+
>Handler : Symbol(Handler, Decl(source.js, 0, 0), Decl(source.js, 7, 1))
35+
36+
module.exports.Strings = Strings
37+
>module.exports.Strings : Symbol(Strings)
38+
>module.exports : Symbol(Strings, Decl(source.js, 14, 25))
39+
>module : Symbol(module, Decl(source.js, 12, 1))
40+
>exports : Symbol("tests/cases/conformance/jsdoc/declarations/source", Decl(source.js, 0, 0))
41+
>Strings : Symbol(Strings, Decl(source.js, 14, 25))
42+
>Strings : Symbol(Strings, Decl(source.js, 9, 5))
43+
44+
/**
45+
* @typedef {Object} HandlerOptions
46+
* @property {String} name
47+
* Should be able to export a type alias at the same time.
48+
*/
49+

‎tests/baselines/reference/jsDeclarationsClassStatic.types

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
=== tests/cases/conformance/jsdoc/declarations/source.js ===
2+
class Handler {
3+
>Handler : Handler
4+
5+
static get OPTIONS() {
6+
>OPTIONS : number
7+
8+
return 1;
9+
>1 : 1
10+
}
11+
12+
process() {
13+
>process : () => void
14+
}
15+
}
16+
Handler.statische = function() { }
17+
>Handler.statische = function() { } : () => void
18+
>Handler.statische : () => void
19+
>Handler : typeof Handler
20+
>statische : () => void
21+
>function() { } : () => void
22+
23+
const Strings = {
24+
>Strings : { a: string; b: string; }
25+
>{ a: "A", b: "B"} : { a: string; b: string; }
26+
27+
a: "A",
28+
>a : string
29+
>"A" : "A"
30+
31+
b: "B"
32+
>b : string
33+
>"B" : "B"
34+
}
35+
36+
module.exports = Handler;
37+
>module.exports = Handler : typeof Handler
38+
>module.exports : typeof Handler
39+
>module : { "\"tests/cases/conformance/jsdoc/declarations/source\"": typeof Handler; }
40+
>exports : typeof Handler
41+
>Handler : typeof Handler
42+
43+
module.exports.Strings = Strings
44+
>module.exports.Strings = Strings : { a: string; b: string; }
45+
>module.exports.Strings : { a: string; b: string; }
46+
>module.exports : typeof Handler
47+
>module : { "\"tests/cases/conformance/jsdoc/declarations/source\"": typeof Handler; }
48+
>exports : typeof Handler
49+
>Strings : { a: string; b: string; }
50+
>Strings : { a: string; b: string; }
51+
52+
/**
53+
* @typedef {Object} HandlerOptions
54+
* @property {String} name
55+
* Should be able to export a type alias at the same time.
56+
*/
57+

‎tests/cases/conformance/jsdoc/declarations/jsDeclarationsClassStatic.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// @allowJs: true
2+
// @checkJs: true
3+
// @target: es5
4+
// @outDir: ./out
5+
// @declaration: true
6+
// @filename: source.js
7+
class Handler {
8+
static get OPTIONS() {
9+
return 1;
10+
}
11+
12+
process() {
13+
}
14+
}
15+
Handler.statische = function() { }
16+
const Strings = {
17+
a: "A",
18+
b: "B"
19+
}
20+
21+
module.exports = Handler;
22+
module.exports.Strings = Strings
23+
24+
/**
25+
* @typedef {Object} HandlerOptions
26+
* @property {String} name
27+
* Should be able to export a type alias at the same time.
28+
*/

0 commit comments

Comments
 (0)