fix(compiler): only create one class member when transforming `@Eleme…

…nt()` decorators (#4547) * only create one class member for an `@Element()` * add some regression tests
ionic-team · Jul 14, 2023 · 13fac03 · 13fac03
1 parent e11ac0e
commit 13fac03
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 22 deletions.
diff --git a/src/compiler/transformers/component-lazy/lazy-element-getter.ts b/src/compiler/transformers/component-lazy/lazy-element-getter.ts
@@ -14,20 +14,35 @@ export const addLazyElementGetter = (
   if (cmp.elementRef) {
     addCoreRuntimeApi(moduleFile, RUNTIME_APIS.getElement);
 
-    classMembers.push(
-      ts.factory.createGetAccessorDeclaration(
-        undefined,
-        cmp.elementRef,
-        [],
-        undefined,
-        ts.factory.createBlock([
-          ts.factory.createReturnStatement(
-            ts.factory.createCallExpression(ts.factory.createIdentifier(GET_ELEMENT), undefined, [
-              ts.factory.createThis(),
-            ])
-          ),
-        ])
-      )
+    // Create the getter that will be used in the transformed class declaration
+    const getter = ts.factory.createGetAccessorDeclaration(
+      undefined,
+      cmp.elementRef,
+      [],
+      undefined,
+      ts.factory.createBlock([
+        ts.factory.createReturnStatement(
+          ts.factory.createCallExpression(ts.factory.createIdentifier(GET_ELEMENT), undefined, [
+            ts.factory.createThis(),
+          ])
+        ),
+      ])
     );
+
+    // Find the index in the class members array that correlates with the element
+    // ref identifier we have
+    const index = classMembers.findIndex(
+      (member) =>
+        member.kind === ts.SyntaxKind.PropertyDeclaration && (member.name as any)?.escapedText === cmp.elementRef
+    );
+
+    // Index should never not be a valid integer, but we'll be safe just in case.
+    // If the index is valid, we'll overwrite the existing class member with the getter
+    // so we don't create multiple members with the same identifier
+    if (index >= 0) {
+      classMembers[index] = getter;
+    } else {
+      classMembers.push(getter);
+    }
   }
 };
diff --git a/src/compiler/transformers/component-native/native-element-getter.ts b/src/compiler/transformers/component-native/native-element-getter.ts
@@ -7,14 +7,30 @@ export const addNativeElementGetter = (classMembers: ts.ClassElement[], cmp: d.C
   // is transformed into:
   // get element() { return this; }
   if (cmp.elementRef) {
-    classMembers.push(
-      ts.factory.createGetAccessorDeclaration(
-        undefined,
-        cmp.elementRef,
-        [],
-        undefined,
-        ts.factory.createBlock([ts.factory.createReturnStatement(ts.factory.createThis())])
-      )
+    // Create the getter that will be used in the transformed class declaration
+    const getter = ts.factory.createGetAccessorDeclaration(
+      undefined,
+      cmp.elementRef,
+      [],
+      undefined,
+      ts.factory.createBlock([ts.factory.createReturnStatement(ts.factory.createThis())])
     );
+
+    ts.SyntaxKind.AmpersandToken;
+    // Find the index in the class members array that correlates with the element
+    // ref identifier we have
+    const index = classMembers.findIndex(
+      (member) =>
+        member.kind === ts.SyntaxKind.PropertyDeclaration && (member.name as any)?.escapedText === cmp.elementRef
+    );
+
+    // Index should never not be a valid integer, but we'll be safe just in case.
+    // If the index is valid, we'll overwrite the existing class member with the getter
+    // so we don't create multiple members with the same identifier
+    if (index >= 0) {
+      classMembers[index] = getter;
+    } else {
+      classMembers.push(getter);
+    }
   }
 };
diff --git a/src/compiler/transformers/test/lazy-component.spec.ts b/src/compiler/transformers/test/lazy-component.spec.ts
@@ -33,4 +33,33 @@ describe('lazy-component', () => {
     expect(t.outputText).toContain(`import { registerInstance as __stencil_registerInstance } from "@stencil/core"`);
     expect(t.outputText).toContain(`__stencil_registerInstance(this, hostRef)`);
   });
+
+  it('adds a getter for an @Element() reference', () => {
+    const compilerCtx = mockCompilerCtx();
+    const transformOpts: d.TransformOptions = {
+      coreImportPath: '@stencil/core',
+      componentExport: 'lazy',
+      componentMetadata: null,
+      currentDirectory: '/',
+      proxy: null,
+      style: 'static',
+      styleImportData: null,
+    };
+
+    const code = `
+      @Component({
+        tag: 'cmp-a'
+      })
+      export class CmpA {
+        @Element() el: HtmlElement;
+      }
+    `;
+
+    const transformer = lazyComponentTransform(compilerCtx, transformOpts);
+
+    const t = transpileModule(code, null, compilerCtx, [], [transformer]);
+
+    expect(t.outputText).toContain(`get el() { return __stencil_getElement(this); } };`);
+    expect(t.outputText).not.toContain(`el;`);
+  });
 });
diff --git a/src/compiler/transformers/test/native-constructor.spec.ts b/src/compiler/transformers/test/native-constructor.spec.ts
@@ -67,5 +67,23 @@ describe('nativeComponentTransform', () => {
       );
       expect(transpiledModule.outputText).toContain(`this.__attachShadow()`);
     });
+
+    it('adds a getter for an @Element() reference', () => {
+      const code = `
+        @Component({
+          tag: 'cmp-a'
+        })
+        export class CmpA {
+          @Element() el: HtmlElement;
+        }
+      `;
+
+      const transformer = nativeComponentTransform(compilerCtx, transformOpts);
+
+      const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);
+
+      expect(transpiledModule.outputText).toContain(`get el() { return this; }`);
+      expect(transpiledModule.outputText).not.toContain(`el;`);
+    });
   });
 });