bug: proxyCustomElement causing infinite loops with react testing library #3434
Comments
|
I've confirmed that this is a bug, although it's not clear yet the root cause is here. I don't know if this is a bug in an external library, a result of wrapping components using the react output targets, etc. Labeling as a bug to get this ingested into our backlog |
rwaskiewicz
added
the
Bug: Validated
|
Hey there @rwaskiewicz, I am happy to report that the problem is due a bug in We can now close this bug ticket as it can be fixed by doing |
|
@vincenthongzy "@ionic/core": "6.2.8" |
|
Any update here? It's been quiet for a while. I tried the nwsapi upgrade, and it did not solve the issue. |
|
An additional observation: the |
|
This critical issue still exists. A possible solution was suggested in the linked issue: ionic-team/ionic-framework#25334 (comment) |
|
Hello, @rwaskiewicz. How do you think there are any blockers that stop this pull request from merge? According to the previous comment, it should solve the problem. |
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 defined 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.
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.
|
Hello all, I just published a dev build of Stencil with a possible fix for this issue. You can install it like this: npm install --save-dev @stencil/core@3.2.0-dev.1679495718.abfe085it would be a tremendous help if you could try that out and report back whether that fixes the issue for you - I hope it does! |
For me, this fix did not solve the problem. But imho, I think that the problem does not seem to look like an infinite loop. If you want to reproduce it, then it's very easy to do so. It is necessary to create an blank ionic application (from tabs template) according to the instructions from the official tutorial. After that, add the following test to the project: In this case, the test will fail, and that is ok. However, instead of the printed html, there will be a long json text on the screen. If you want to get an even more strange error, uncomment the line where App is used instead of IonButton. The planned result will be so big that json printing will be replaced by the problem of html dumping. In this case, the error will look similar to the infinite loop problem |
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.
|
Did not solve for me, although I'm not certain I am including it correctly. Test that causes bad serialization looks like this: import { IonSkeletonText } from '@ionic/react'
import { waitForIonicReact } from '@ionic/react-test-utils'
import { render } from '@testing-library/react'
describe('SkeletonForm', () => {
it('should render a skeleton form', async () => {
const { debug } = render(<IonSkeletonText />)
await waitForIonicReact()
debug()
})
})To integrate the above fix, I'm:
|
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.
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.
|
The fix for this issue has been included in today's v3.2.1 release of Stencil. As a result, I'm going to close this issue. Please note that if you are encountering this issue while using the Ionic Framework, a new patch release of the Ionic Framework will need to be released that uses Stencil v3.2.1 in order to be resolved on the Framework side (see issue ionic-team/ionic-framework#25334). |
|
Unfortunately we are still running into this issue, even after building our component library with v3.2.1 release of Stencil. I checked that the inlined class expression is no longer anonymous as described in PR #4188 and it looks like it should look: const LdInputMessage$1 = /*@__PURE__*/ proxyCustomElement(class LdInputMessage extends HTMLElement {However, the issue still occurs in a specific test case. I haven't been able to figure out the crucial difference to other test cases / components yet. I'm attaching a link to an application on Stackblitz which contains a test that leads to the above issue, if you uncomment the last two lines of the second test: https://stackblitz.com/github/emdgroup-liquid/liquid-sandbox-react-tailwind?file=src%2FApp.test.tsx%3AL34 It would be great, if you could have a second look at this issue @alicewriteswrongs @rwaskiewicz. 🙏 |
|
@borisdiakur Thanks! Can you please create a new issue with your reproduction case there for us to properly track it? |
|
@rwaskiewicz Yes, I will. Or maybe not. 😅 I actually managed to dig a little further in the last hour, by removing all components from the app. This resulted in a different but seemingly related issue (Maximum call stack size exceeded in picocolors used by vitest for printing error messages). The actual error message (quite huge - see screenshot) says that the testing library is not able to find the element with text that I'm looking for in the test.
Since I'm not even using web components in the test right now, I think this issue is not related to the one documented above (although at first sight it seemed so). TLDR: For now I think we're good and I won't need to open a new issue (at least not for Stencil). However, if I put the Web Components back in place, and run into a similar problem again, I will create a new one. Thanks a lot 🙏 |
|
Hello. We've a complete design system developed using stencil 2.x and a consumer found out this issue. Looking into the commit message I understand there is no breaking change, so I would appreciate it very much if this could be done. I can provide a PR if you agree on that |
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.
|
Hey @alicewriteswrongs sorry taking so much time to answer. I tried this package and the issue is still happening, there might be other things in v3 that help fixing it. Thanks again for providing the test package to try this out! |
Prerequisites
Stencil Version
2.17.0
Current Behavior
Rendering multiple Stencil components wrapped as React components causes an
PrettyFormatPluginErrorerror when used with@testing-library/react. This error only reproduces in a test environment.Expected Behavior
I expect there to be no errors when running the test.
Steps to Reproduce
This script does the following things:
a) Builds and packs the base Stencil component library in
sample-component-libraryb) Uses the packed
sample-component-libraryto build React wrappers for the Stencil components insample-component-library-react.c) Uses the packed
sample-component-library-reactto consume the React wrappers in a React application namedsample-app.sample-app, runnpm run test. Observe that the following error is logged:I did more debugging in
sample-app/node_modules/sample-component-library/dist/components/my-component.jsand found that the following changes "resolve" the issue. In reality, this just causes the relevant Stencil code to never get loaded, but it seems that something in theproxyCustomElementcall could be causing this.Code Reproduction URL
https://github.com/vincenthongzy/stencil-repro
Additional Information
This issue was discovered by an Ionic Framework user. There is more context in ionic-team/ionic-framework#25334
There may be some cases where infinite recursion could happen in the pretty printing package (ionic-team/ionic-framework#25334 (comment)), so it could also be the case that Stencil is triggering a bug in the pretty printing package instead.
The text was updated successfully, but these errors were encountered: