Skip to content

Commit 6dbe9a5

Browse files
authored
fix(compiler): handle ts 5.0 static members (#4447)
this commit fixes a bug where static class members that are intialized on a stencil component result in 2 `export` statements of a component class being generated. as a result of 2 `ExportDeclaration`s being created, the compilation process will fail. this fix involves detecting initialized static members of a class before they go through the transpilation phase, because a class can be transformed in such a way that we lose the ability to determine if something like the following was originally authored: ```ts class MyComponent { static myVar = 1; } ``` as a result, whether or not this static initializer exists or not is stuffed onto the component's metadata. this allows us to store if an additional transformation is needed or not after transpilation has occurred. specifcially, should we add an `export` keyword to a modified class declaration in the form: ```ts // determine if this `VariableDeclaration` needs an `export` const MyComponent = class {} ``` Additional Information Consider a TypeScript class that: - has a static member that is defined on the class - is exported from the file in which it is defined ```ts export class MyComponent { static myVar = 1; } ``` [In TypeScript v4.9 and lower](https://www.typescriptlang.org/play?ts=4.9.5#code/KYDwDg9gTgLgBAYwDYEMDOa4FkCeBhCAW0gDtgT4BvAKAEg0YUYBLBOQnANRSjgF44ARgDc1AL5A) the class is transpiled as such: ```js export class MyComponent { } MyComponent.myVar = 1; ``` [Starting with TypeScript v5.0](https://www.typescriptlang.org/play?ts=5.0.4#code/KYDwDg9gTgLgBAYwDYEMDOa4FkCeBhCAW0gDtgT4BvAKAEg0YUYBLBOQnANRSjgF44ARgDc1AL5A), the class is transpiled like so: ```js class MyComponent { } MyComponent.myVar = 1; export { MyComponent }; ``` Where the `export` occurs separate from the class's declaration: ```diff - export class MyComponent { + class MyComponent { } MyComponent.myVar = 1; + export { MyComponent }; ``` Note this only occurs when both conditions above are met. The following code snippet compiles the same using TypeScript [v4.9](https://www.typescriptlang.org/play?ts=4.9.5#code/KYDwDg9gTgLgBAYwDYEMDOa4FkCeBhCAW0gDtgT4BvAKAF8g) and [v5.0](https://www.typescriptlang.org/play?ts=5.0.4&ssl=2&ssc=2&pln=1&pc=1#code/KYDwDg9gTgLgBAYwDYEMDOa4FkCeBhCAW0gDtgT4BvAKAF8g): ```ts export class MyComponent { } // transpiles to export class MyComponent { } ``` The difference in compilation when both conditions are met leads to a bug in Stencil v3.3.0, the first version of the library which supports TypeScript v5.0. Stencil made the assumption that the `export` keyword continues to be inlined with the class declaration, rather than a new `ExportDeclaration` instance being created. In Stencil v3.3.0, the `export` keyword is added to the generated code: ```ts export class MyComponent {}; MyComponent.myVar = 1; export { MyComponent }; // 2 exports causes an error! ``` Two exports of the class (`MyComponent`) causes errors following the TypeScript compilation step in Stencil: ```console [ ERROR ] Rollup: Parse Error: <FILE_PATH> Duplicate export 'MyComponent' (Note that you need plugins to import files that are not JavaScript) Error parsing: <FILE_PATH> ``` In an ideal world, we delegate inserting `export` to the TypeScript compiler. However, this is not possible as the `ExportDeclaration` is _only_ created for classes that are exported and have static members. So while removing Stencil's `export` keyword injection will work for: ```ts export class MyComponent { static myVar = 1; } // becomes the following on TS 5.0 if we remove Stencil's `export` injection class MyComponent { } MyComponent.myVar = 1; export { MyComponent }; ``` it will not work for: ```ts export class MyComponent { myVar = 1; } // becomes the following on TS 5.0 if we remove Stencil's `export` injection class MyComponent { constructor() { this.myVar = 1; } } ``` as `export` is removed from `MyComponent` in the compilation process. This behavior is controlled by the [`TransformOptions#componentExport` field](https://github.com/ionic-team/stencil/blob/bd1023a67c5224cbae3e445170cb13227a5cf8fb/src/declarations/stencil-public-compiler.ts#L2683). Import injection should only occur when neither of two aforementioned criteria are met: - has a static member that is defined on the class - is exported from the file in which it is defined
1 parent c83c113 commit 6dbe9a5

22 files changed

+450
-18
lines changed

src/compiler/output-targets/dist-custom-elements/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ export const addCustomElementInputs = (
204204

205205
if (cmp.isPlain) {
206206
exp.push(`export { ${importName} as ${exportName} } from '${cmp.sourceFilePath}';`);
207+
// TODO(STENCIL-855): Invalid syntax generation, note the unbalanced left curly brace
207208
indexExports.push(`export { {${exportName} } from '${coreKey}';`);
208209
} else {
209210
// the `importName` may collide with the `exportName`, alias it just in case it does with `importAs`

src/compiler/transformers/component-lazy/lazy-component.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export const updateLazyComponentClass = (
1717
cmp: d.ComponentCompilerMeta
1818
) => {
1919
const members = updateLazyComponentMembers(transformOpts, styleStatements, classNode, moduleFile, cmp);
20-
return updateComponentClass(transformOpts, classNode, classNode.heritageClauses, members);
20+
return updateComponentClass(transformOpts, classNode, classNode.heritageClauses, members, moduleFile);
2121
};
2222

2323
const updateLazyComponentMembers = (

src/compiler/transformers/component-native/native-component.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const updateNativeComponentClass = (
2020
): ts.ClassDeclaration | ts.VariableStatement => {
2121
const heritageClauses = updateNativeHostComponentHeritageClauses(classNode, moduleFile);
2222
const members = updateNativeHostComponentMembers(transformOpts, classNode, moduleFile, cmp);
23-
return updateComponentClass(transformOpts, classNode, heritageClauses, members);
23+
return updateComponentClass(transformOpts, classNode, heritageClauses, members, moduleFile);
2424
};
2525

2626
/**

src/compiler/transformers/component-native/proxy-custom-element-function.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export const proxyCustomElement = (
8484
// update the variable statement containing the updated declaration list
8585
const updatedVariableStatement = ts.factory.updateVariableStatement(
8686
stmt,
87-
[ts.factory.createModifier(ts.SyntaxKind.ExportKeyword)],
87+
stmt.modifiers,
8888
updatedDeclarationList
8989
);
9090

src/compiler/transformers/decorators-to-static/convert-decorators.ts

+15-1
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@ import { augmentDiagnosticWithNode, buildError } from '@utils';
22
import ts from 'typescript';
33

44
import type * as d from '../../../declarations';
5-
import { retrieveTsDecorators, retrieveTsModifiers } from '../transform-utils';
5+
import {
6+
convertValueToLiteral,
7+
createStaticGetter,
8+
retrieveTsDecorators,
9+
retrieveTsModifiers,
10+
} from '../transform-utils';
611
import { componentDecoratorToStatic } from './component-decorator';
12+
import { hasStaticInitializerInClass } from './convert-static-members';
713
import { isDecoratorNamed } from './decorator-utils';
814
import {
915
CLASS_DECORATORS_TO_REMOVE,
@@ -72,6 +78,14 @@ const visitClassDeclaration = (
7278
listenDecoratorsToStatic(diagnostics, decoratedMembers, filteredMethodsAndFields);
7379
}
7480

81+
// Handle static members that are initialized
82+
const hasStaticMembersWithInit = classMembers.some(hasStaticInitializerInClass);
83+
if (hasStaticMembersWithInit) {
84+
filteredMethodsAndFields.push(
85+
createStaticGetter('stencilHasStaticMembersWithInit', convertValueToLiteral(hasStaticMembersWithInit))
86+
);
87+
}
88+
7589
// We call the `handleClassFields` method which handles transforming any
7690
// class fields, removing them from the class and adding statements to the
7791
// class' constructor which instantiate them there instead.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import ts from 'typescript';
2+
3+
/**
4+
* Helper function to detect if a class element fits the following criteria:
5+
* - It is a property declaration (e.g. `foo`)
6+
* - It has an initializer (e.g. `foo *=1*`)
7+
* - The property declaration has the `static` modifier on it (e.g. `*static* foo =1`)
8+
* @param classElm the class member to test
9+
* @returns true if the class member fits the above criteria, false otherwise
10+
*/
11+
export const hasStaticInitializerInClass = (classElm: ts.ClassElement): boolean => {
12+
return (
13+
ts.isPropertyDeclaration(classElm) &&
14+
classElm.initializer !== undefined &&
15+
Array.isArray(classElm.modifiers) &&
16+
classElm.modifiers!.some((modifier) => modifier.kind === ts.SyntaxKind.StaticKeyword)
17+
);
18+
};

src/compiler/transformers/remove-static-meta-properties.ts

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export const removeStaticMetaProperties = (classNode: ts.ClassDeclaration): ts.C
2424
});
2525
};
2626

27+
// TODO(STENCIL-856): Move these properties to constants for better type safety within the codebase
2728
/**
2829
* A list of static getter names that are specific to Stencil to exclude from a class's member list
2930
*/
@@ -37,6 +38,7 @@ const REMOVE_STATIC_GETTERS = new Set([
3738
'methods',
3839
'states',
3940
'originalStyleUrls',
41+
'stencilHasStaticMembersWithInit',
4042
'styleMode',
4143
'style',
4244
'styles',

src/compiler/transformers/static-to-meta/component.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,14 @@ export const parseStaticComponentMeta = (
5454
const docs = serializeSymbol(typeChecker, symbol);
5555
const isCollectionDependency = moduleFile.isCollectionDependency;
5656
const encapsulation = parseStaticEncapsulation(staticMembers);
57-
5857
const cmp: d.ComponentCompilerMeta = {
5958
tagName: tagName,
6059
excludeFromCollection: moduleFile.excludeFromCollection,
6160
isCollectionDependency,
6261
componentClassName: cmpNode.name ? cmpNode.name.text : '',
6362
elementRef: parseStaticElementRef(staticMembers),
6463
encapsulation,
64+
hasStaticInitializedMember: getStaticValue(staticMembers, 'stencilHasStaticMembersWithInit') ?? false,
6565
shadowDelegatesFocus: parseStaticShadowDelegatesFocus(encapsulation, staticMembers),
6666
properties: parseStaticProps(staticMembers),
6767
virtualProperties: parseVirtualProps(docs),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import ts from 'typescript';
2+
3+
import { hasStaticInitializerInClass } from '../decorators-to-static/convert-static-members';
4+
5+
describe('convert-static-members', () => {
6+
describe('hasStaticInitializerInClass', () => {
7+
it('returns true for a static property with an initializer', () => {
8+
const classWithStaticMembers = ts.factory.createClassDeclaration(
9+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
10+
ts.factory.createIdentifier('ClassWithStaticMember'),
11+
undefined,
12+
undefined,
13+
[
14+
ts.factory.createPropertyDeclaration(
15+
[ts.factory.createToken(ts.SyntaxKind.StaticKeyword)],
16+
ts.factory.createIdentifier('propertyName'),
17+
undefined,
18+
undefined,
19+
ts.factory.createStringLiteral('initial value')
20+
),
21+
]
22+
);
23+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(true);
24+
});
25+
26+
it('returns true for a private static property with an initializer', () => {
27+
const classWithStaticMembers = ts.factory.createClassDeclaration(
28+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
29+
ts.factory.createIdentifier('ClassWithStaticMember'),
30+
undefined,
31+
undefined,
32+
[
33+
ts.factory.createPropertyDeclaration(
34+
[ts.factory.createToken(ts.SyntaxKind.PrivateKeyword), ts.factory.createToken(ts.SyntaxKind.StaticKeyword)],
35+
ts.factory.createIdentifier('propertyName'),
36+
undefined,
37+
undefined,
38+
ts.factory.createStringLiteral('initial value')
39+
),
40+
]
41+
);
42+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(true);
43+
});
44+
45+
it('returns true for a static property with an initializer with multiple members', () => {
46+
const classWithStaticMembers = ts.factory.createClassDeclaration(
47+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
48+
ts.factory.createIdentifier('ClassWithStaticAndNonStaticMembers'),
49+
undefined,
50+
undefined,
51+
[
52+
ts.factory.createPropertyDeclaration(
53+
undefined,
54+
ts.factory.createIdentifier('nonStaticProperty'),
55+
undefined,
56+
undefined,
57+
ts.factory.createStringLiteral('some value')
58+
),
59+
ts.factory.createPropertyDeclaration(
60+
[ts.factory.createToken(ts.SyntaxKind.StaticKeyword)],
61+
ts.factory.createIdentifier('propertyName'),
62+
undefined,
63+
undefined,
64+
ts.factory.createStringLiteral('initial value')
65+
),
66+
]
67+
);
68+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(true);
69+
});
70+
71+
it('returns false for a class without any members', () => {
72+
const classWithStaticMembers = ts.factory.createClassDeclaration(
73+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
74+
ts.factory.createIdentifier('ClassWithNoMembers'),
75+
undefined,
76+
undefined,
77+
[] // no members for this class
78+
);
79+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(false);
80+
});
81+
82+
it('returns false for a static property without an initializer', () => {
83+
const classWithStaticMembers = ts.factory.createClassDeclaration(
84+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
85+
ts.factory.createIdentifier('ClassWithUninitializedStaticMember'),
86+
undefined,
87+
undefined,
88+
[
89+
ts.factory.createPropertyDeclaration(
90+
[ts.factory.createToken(ts.SyntaxKind.StaticKeyword)],
91+
ts.factory.createIdentifier('propertyName'),
92+
undefined,
93+
undefined,
94+
undefined // the initializer is false
95+
),
96+
]
97+
);
98+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(false);
99+
});
100+
101+
it('returns false for a private static property without an initializer', () => {
102+
const classWithStaticMembers = ts.factory.createClassDeclaration(
103+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
104+
ts.factory.createIdentifier('ClassWithUninitializedStaticMember'),
105+
undefined,
106+
undefined,
107+
[
108+
ts.factory.createPropertyDeclaration(
109+
[ts.factory.createToken(ts.SyntaxKind.PrivateKeyword), ts.factory.createToken(ts.SyntaxKind.StaticKeyword)],
110+
ts.factory.createIdentifier('propertyName'),
111+
undefined,
112+
undefined,
113+
undefined // the initializer is false
114+
),
115+
]
116+
);
117+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(false);
118+
});
119+
120+
it('returns false for a modified property with an initializer', () => {
121+
const classWithStaticMembers = ts.factory.createClassDeclaration(
122+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
123+
ts.factory.createIdentifier('ClassWithNonStaticMember'),
124+
undefined,
125+
undefined,
126+
[
127+
ts.factory.createPropertyDeclaration(
128+
[ts.factory.createToken(ts.SyntaxKind.PrivateKeyword)], // the property is declared as private
129+
ts.factory.createIdentifier('propertyName'),
130+
undefined,
131+
undefined,
132+
ts.factory.createStringLiteral('initial value')
133+
),
134+
]
135+
);
136+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(false);
137+
});
138+
139+
it('returns false for an unmodified property with an initializer', () => {
140+
const classWithStaticMembers = ts.factory.createClassDeclaration(
141+
[ts.factory.createToken(ts.SyntaxKind.ExportKeyword)],
142+
ts.factory.createIdentifier('ClassWithUnmodifiedMembers'),
143+
undefined,
144+
undefined,
145+
[
146+
ts.factory.createPropertyDeclaration(
147+
undefined, // the property declaration has no modifiers
148+
ts.factory.createIdentifier('propertyName'),
149+
undefined,
150+
undefined,
151+
ts.factory.createStringLiteral('initial value')
152+
),
153+
]
154+
);
155+
expect(classWithStaticMembers.members.some(hasStaticInitializerInClass)).toBe(false);
156+
});
157+
});
158+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { transpileModule } from './transpile';
2+
3+
describe('parse static members', () => {
4+
it('places a static getter on the component', () => {
5+
const t = transpileModule(`
6+
@Component({tag: 'cmp-a'})
7+
export class CmpA {
8+
static myStatic = 'a value';
9+
10+
render() {
11+
return <div>Hello, I have {CmpA.myStatic}</div>
12+
}
13+
}
14+
`);
15+
16+
expect(t.outputText.includes('static get stencilHasStaticMembersWithInit() { return true; }')).toBe(true);
17+
});
18+
});

src/compiler/transformers/test/proxy-custom-element-function.spec.ts

+16-3
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,19 @@ describe('proxy-custom-element-function', () => {
8181
const transformer = proxyCustomElement(compilerCtx, transformOpts);
8282
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);
8383

84+
expect(formatCode(transpiledModule.outputText)).toContain(
85+
formatCode(
86+
`const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true);`
87+
)
88+
);
89+
});
90+
91+
it('wraps an exported class initializer in a proxyCustomElement call', () => {
92+
const code = `export const ${componentClassName} = class extends HTMLElement {};`;
93+
94+
const transformer = proxyCustomElement(compilerCtx, transformOpts);
95+
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);
96+
8497
expect(formatCode(transpiledModule.outputText)).toContain(
8598
formatCode(
8699
`export const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true);`
@@ -97,7 +110,7 @@ describe('proxy-custom-element-function', () => {
97110

98111
expect(formatCode(transpiledModule.outputText)).toContain(
99112
formatCode(
100-
`export const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true);`
113+
`const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true);`
101114
)
102115
);
103116
});
@@ -110,7 +123,7 @@ describe('proxy-custom-element-function', () => {
110123

111124
expect(formatCode(transpiledModule.outputText)).toContain(
112125
formatCode(
113-
`export const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true), foo = 'hello world!';`
126+
`const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true), foo = 'hello world!';`
114127
)
115128
);
116129
});
@@ -123,7 +136,7 @@ describe('proxy-custom-element-function', () => {
123136

124137
expect(formatCode(transpiledModule.outputText)).toContain(
125138
formatCode(
126-
`export const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true), bar = 'goodbye?';`
139+
`const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true), bar = 'goodbye?';`
127140
)
128141
);
129142
});

src/compiler/transformers/test/transpile.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,19 @@ export function transpileModule(
175175
*/
176176
const prettifyTSOutput = (tsOutput: string): string => tsOutput.replace(/\s+/gm, ' ');
177177

178-
export function getStaticGetter(output: string, prop: string) {
179-
const toEvaluate = `return ${output.replace('export', '')}`;
178+
/**
179+
* Helper function for tests that converts stringified JavaScript to a runtime value.
180+
* A value from the generated JavaScript is returned based on the provided property name.
181+
* @param stringifiedJs the stringified JavaScript
182+
* @param propertyName the property name to pull off the generated JavaScript
183+
* @returns the value associated with the provided property name. Returns undefined if an error occurs while converting
184+
* the stringified JS to JavaScript, or if the property does not exist on the generated JavaScript.
185+
*/
186+
export function getStaticGetter(stringifiedJs: string, propertyName: string): string | void {
187+
const toEvaluate = `return ${stringifiedJs.replace('export', '')}`;
180188
try {
181189
const Obj = new Function(toEvaluate);
182-
return Obj()[prop];
190+
return Obj()[propertyName];
183191
} catch (e) {
184192
console.error(e);
185193
console.error(toEvaluate);

0 commit comments

Comments
 (0)