test(ReactElementToJsxString): Add test when dealing with element inside prop object

[thomasbertet]

Following issue #307 & PR #309

I added two tests:

• One is to cover the one-deep level of nesting, ie. an element inside a prop object.
• One is to over deep-nested level, .ie. an element inside a prop object inside another element inside a prop object ... you get the idea 💃

Please tell me if anything is unclear or needs to follow certain rules. I tried to make them just as the others but I might have missed some.

@vvo Thanks for your time! Would love to see this fix released

[SachsKaylee]

Hello @thomasbertet,

thank you very much for your contribution.

Unfortunately this test does not cover the issue in #307. As can be seen in this code snippet, the error only occurs if the React element to be transformed into a string was created by another React component.

This is because React adds the "_owner" key to these elements pointing to the owning React component, which can cause an infinite loop.
If the React component was created outside of another components lifecycle, this key does not exist, and thus the stack overflow will not occur.

The unit tests are ran by Jest which is separate from the React lifecycle, thus elements created in the unit tests do not have the "_owner" key. This can be verified by (temporarily) commenting out the fix of #307 and then running the tests again.

I'm not sure if we can properly test this using our current testing setup without using a virtual dom.

[SachsKaylee]

I'm open to suggestions on how to approach this.
Adding a virtual dom to the testing setup is possible, but this sounds like a very heavyweight solution for a single unit test.

Do you have other ideas?

[thomasbertet]

Oh I see... I thought it was quite easier. Sorry about the misunderstanding. I could have done the check you did by commenting out the fix.
Let me try some and I will get back to you.

[thomasbertet]

@PatrickSachs I added a proper test case for the issue. Please try it if you have time.
As you mentioned, I had to add Enzyme to the game. I know that sound quite heavy, but might be useful in the future to have it there ?
TBH, I did not take the time to try anything else. I was aiming for a quick result.

Please tell me what you think, but IMHO this might be good enough :)

[SachsKaylee]

Without having looked at the tests too much yet, I'm not a fan of adding this .spec.js file in a different directory than the other ones. I think the test either belongs in index.spec.js or sortObject.spec.js.

The tests fail and pass as expected, but I'll have a more in depth look at it.

[thomasbertet]

@PatrickSachs I moved the test file.

[SachsKaylee]

This looks good to me. Adding Enzyme for currently (just) a single unit tests is a big heavy, but I believe it will become useful for more tests in time.

Unless any more issues crop up related to this I'll create a new release tomorrow. @samwalshnz