Skip to content

Commit

Permalink
fix(test): fix infinite loops w/ react and @testing-library/dom
Browse files Browse the repository at this point in the history
This fixes an issue (documented in #3434) where when using
`@testing-libary/dom` to test a Stencil component wrapped with the React
framework wrappers could produce an infinite loop that would cause the
tests to fail.

The issue relates to an assumption that `@testing-library/dom` makes
about the `.name` property on the constructor for a custom element. In
particular, `@testing-library/dom` expects the property to be truthy
here:

https://github.com/testing-library/dom-testing-library/blob/fb069c93983bc0300a6e1c91bdec5bf9443b5286/src/DOMElementFilter.ts#L198

When building with the `dist-custom-elements` output target we create an
anonymous class expression and inline it into a call in the emitted JS
to `proxyCustomElement`, like this:

```js
const MyComponent$1 = /*@__PURE__*/ proxyCustomElement(
  class extends HTMLElement {
    ...
  },
  [1, "my-component", {}]
);
```

We made a change (#3248) to fix an issue (#3191) with webpack
treeshaking where if we didn't inline an anonymous class expression like
this we would get improper tree shaking in webpack.

One consequence, however, of an _anonymous_ inline class expression is
that the `.name` property on its constructor is going to be `""`, which
fails the false-ey test in `@testing-library/dom` referenced above.

So in order to fix the issue we can simply insert a name so that the
inlined class expression is no longer anonymous, like so:

```js
const MyComponent$1 = /*@__PURE__*/ proxyCustomElement(
  class MyComponent extends HTMLElement {
    ...
  },
  [1, "my-component", {}]
);
```

This fixes the issue with infinite loops while testing with the React
wrapper. Additionally, using the reproduction case provided for #3191 we
can confirm that this does not cause a regression with respect the
previous fix for the webpack treeshaking issue.
  • Loading branch information
alicewriteswrongs committed Mar 24, 2023
1 parent 69176f8 commit 5ebe2a9
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 35 deletions.
21 changes: 12 additions & 9 deletions src/compiler/transformers/add-component-meta-proxy.ts
Expand Up @@ -47,24 +47,27 @@ export const createComponentMetadataProxy = (compilerMeta: d.ComponentCompilerMe
};

/**
* Create a call expression for wrapping a component represented as an anonymous class in a proxy. This call expression
* takes a form:
* Create a call expression for wrapping a component represented as a class
* expression in a proxy. This call expression takes the form:
*
* ```ts
* PROXY_CUSTOM_ELEMENT(Clazz, Metadata);
* ```
*
* where
* - `PROXY_CUSTOM_ELEMENT` is a Stencil internal identifier that will be replaced with the name of the actual function
* name at compile name
* - `Clazz` is an anonymous class to be proxied
* - `PROXY_CUSTOM_ELEMENT` is a Stencil internal identifier that will be
* replaced with the name of the actual function name at compile name
* - `Clazz` is a class expression to be proxied
* - `Metadata` is the compiler metadata associated with the Stencil component
*
* @param compilerMeta compiler metadata associated with the component to be wrapped in a proxy
* @param clazz the anonymous class to proxy
* @param compilerMeta compiler metadata associated with the component to be
* wrapped in a proxy
* @param clazz the class expression to proxy
* @returns the generated call expression
*/
export const createAnonymousClassMetadataProxy = (
export const createClassMetadataProxy = (
compilerMeta: d.ComponentCompilerMeta,
clazz: ts.Expression
clazz: ts.ClassExpression
): ts.CallExpression => {
const compactMeta: d.ComponentRuntimeMetaCompact = formatComponentRuntimeMeta(compilerMeta, true);
const literalMeta = convertValueToLiteral(compactMeta);
Expand Down
@@ -1,7 +1,7 @@
import ts from 'typescript';

import type * as d from '../../../declarations';
import { createAnonymousClassMetadataProxy } from '../add-component-meta-proxy';
import { createClassMetadataProxy } from '../add-component-meta-proxy';
import { addImports } from '../add-imports';
import { RUNTIME_APIS } from '../core-runtime-apis';
import { getModuleFromSourceFile } from '../transform-utils';
Expand Down Expand Up @@ -47,8 +47,22 @@ export const proxyCustomElement = (
continue;
}

// to narrow the type of `declaration.initializer` to `ts.ClassExpression`
if (!ts.isClassExpression(declaration.initializer)) {
continue;
}

const renamedClassExpression = ts.factory.updateClassExpression(
declaration.initializer,
ts.getModifiers(declaration.initializer),
ts.factory.createIdentifier(principalComponent.componentClassName),
declaration.initializer.typeParameters,
declaration.initializer.heritageClauses,
declaration.initializer.members
);

// wrap the Stencil component's class declaration in a component proxy
const proxyCreationCall = createAnonymousClassMetadataProxy(principalComponent, declaration.initializer);
const proxyCreationCall = createClassMetadataProxy(principalComponent, renamedClassExpression);
ts.addSyntheticLeadingComment(proxyCreationCall, ts.SyntaxKind.MultiLineCommentTrivia, '@__PURE__', false);

// update the component's variable declaration to use the new initializer
Expand Down
14 changes: 7 additions & 7 deletions src/compiler/transformers/test/add-component-meta-proxy.spec.ts
Expand Up @@ -3,12 +3,12 @@ import ts from 'typescript';
import { stubComponentCompilerMeta } from '../../../compiler/types/tests/ComponentCompilerMeta.stub';
import type * as d from '../../../declarations';
import * as FormatComponentRuntimeMeta from '../../../utils/format-component-runtime-meta';
import { createAnonymousClassMetadataProxy } from '../add-component-meta-proxy';
import { createClassMetadataProxy } from '../add-component-meta-proxy';
import { HTML_ELEMENT } from '../core-runtime-apis';
import * as TransformUtils from '../transform-utils';

describe('add-component-meta-proxy', () => {
describe('createAnonymousClassMetadataProxy()', () => {
describe('createClassMetadataProxy()', () => {
let classExpr: ts.ClassExpression;
let htmlElementHeritageClause: ts.HeritageClause;
let literalMetadata: ts.StringLiteral;
Expand Down Expand Up @@ -51,33 +51,33 @@ describe('add-component-meta-proxy', () => {
});

it('returns a call expression', () => {
const result: ts.CallExpression = createAnonymousClassMetadataProxy(stubComponentCompilerMeta(), classExpr);
const result: ts.CallExpression = createClassMetadataProxy(stubComponentCompilerMeta(), classExpr);

expect(ts.isCallExpression(result)).toBe(true);
});

it('wraps the initializer in PROXY_CUSTOM_ELEMENT', () => {
const result: ts.CallExpression = createAnonymousClassMetadataProxy(stubComponentCompilerMeta(), classExpr);
const result: ts.CallExpression = createClassMetadataProxy(stubComponentCompilerMeta(), classExpr);

expect((result.expression as ts.Identifier).escapedText).toBe('___stencil_proxyCustomElement');
});

it("doesn't add any type arguments to the call", () => {
const result: ts.CallExpression = createAnonymousClassMetadataProxy(stubComponentCompilerMeta(), classExpr);
const result: ts.CallExpression = createClassMetadataProxy(stubComponentCompilerMeta(), classExpr);

expect(result.typeArguments).toHaveLength(0);
});

it('adds the correct arguments to the PROXY_CUSTOM_ELEMENT call', () => {
const result: ts.CallExpression = createAnonymousClassMetadataProxy(stubComponentCompilerMeta(), classExpr);
const result: ts.CallExpression = createClassMetadataProxy(stubComponentCompilerMeta(), classExpr);

expect(result.arguments).toHaveLength(2);
expect(result.arguments[0]).toBe(classExpr);
expect(result.arguments[1]).toBe(literalMetadata);
});

it('includes the heritage clause', () => {
const result: ts.CallExpression = createAnonymousClassMetadataProxy(stubComponentCompilerMeta(), classExpr);
const result: ts.CallExpression = createClassMetadataProxy(stubComponentCompilerMeta(), classExpr);

expect(result.arguments.length).toBeGreaterThanOrEqual(1);
const createdClassExpression = result.arguments[0];
Expand Down
Expand Up @@ -17,9 +17,9 @@ describe('proxy-custom-element-function', () => {
ReturnType<typeof TransformUtils.getModuleFromSourceFile>,
Parameters<typeof TransformUtils.getModuleFromSourceFile>
>;
let createAnonymousClassMetadataProxySpy: jest.SpyInstance<
ReturnType<typeof AddComponentMetaProxy.createAnonymousClassMetadataProxy>,
Parameters<typeof AddComponentMetaProxy.createAnonymousClassMetadataProxy>
let createClassMetadataProxySpy: jest.SpyInstance<
ReturnType<typeof AddComponentMetaProxy.createClassMetadataProxy>,
Parameters<typeof AddComponentMetaProxy.createClassMetadataProxy>
>;

beforeEach(() => {
Expand Down Expand Up @@ -47,20 +47,19 @@ describe('proxy-custom-element-function', () => {
} as d.Module;
});

createAnonymousClassMetadataProxySpy = jest.spyOn(AddComponentMetaProxy, 'createAnonymousClassMetadataProxy');
createAnonymousClassMetadataProxySpy.mockImplementation(
(_compilerMeta: d.ComponentCompilerMeta, clazz: ts.Expression) =>
ts.factory.createCallExpression(
ts.factory.createIdentifier(PROXY_CUSTOM_ELEMENT),
[],
[clazz, ts.factory.createTrue()]
)
createClassMetadataProxySpy = jest.spyOn(AddComponentMetaProxy, 'createClassMetadataProxy');
createClassMetadataProxySpy.mockImplementation((_compilerMeta: d.ComponentCompilerMeta, clazz: ts.Expression) =>
ts.factory.createCallExpression(
ts.factory.createIdentifier(PROXY_CUSTOM_ELEMENT),
[],
[clazz, ts.factory.createTrue()]
)
);
});

afterEach(() => {
getModuleFromSourceFileSpy.mockRestore();
createAnonymousClassMetadataProxySpy.mockRestore();
createClassMetadataProxySpy.mockRestore();
});

describe('proxyCustomElement()', () => {
Expand All @@ -82,7 +81,7 @@ describe('proxy-custom-element-function', () => {
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);

expect(transpiledModule.outputText).toContain(
`export const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class extends HTMLElement {}, true);`
`export const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true);`
);
});

Expand All @@ -94,7 +93,7 @@ describe('proxy-custom-element-function', () => {
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);

expect(transpiledModule.outputText).toContain(
`export const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class extends HTMLElement {}, true);`
`export const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true);`
);
});

Expand All @@ -105,18 +104,18 @@ describe('proxy-custom-element-function', () => {
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);

expect(transpiledModule.outputText).toContain(
`export const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class extends HTMLElement {}, true), foo = 'hello world!';`
`export const ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true), foo = 'hello world!';`
);
});

it('wraps a class initializer properly in the middle of multiple variable declarations', () => {
const code = `const foo = 'hello world!', ${componentClassName} = class extends HTMLElement {}, bar = 'goodbye?'`;
const code = `const foo = 'hello world!', ${componentClassName} = class ${componentClassName} extends HTMLElement {}, bar = 'goodbye?'`;

const transformer = proxyCustomElement(compilerCtx, transformOpts);
const transpiledModule = transpileModule(code, null, compilerCtx, [], [transformer]);

expect(transpiledModule.outputText).toContain(
`export const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class extends HTMLElement {}, true), bar = 'goodbye?';`
`export const foo = 'hello world!', ${componentClassName} = /*@__PURE__*/ __stencil_proxyCustomElement(class ${componentClassName} extends HTMLElement {}, true), bar = 'goodbye?';`
);
});
});
Expand Down

0 comments on commit 5ebe2a9

Please sign in to comment.