Remove hydrate() warning about empty container #10345
Conversation
It used to have false positives for cases where we legitimately don't render anything.
|
|
||
| itRenders('emptyish values', async render => { | ||
| let e = await render(0); | ||
| expect(e.nodeType).toBe(3); |
There was a problem hiding this comment.
What about using Element.TEXT_NODE instead of 3? Is that possible?
There was a problem hiding this comment.
Want to send follow up? We actually already have this as a constant in the same file. But literal is used in more than one test.
There was a problem hiding this comment.
Existing constant is TEXT_NODE_TYPE.
| expect(await render(false)).toBe(null); | ||
| expect(await render(true)).toBe(null); | ||
| expect(await render(undefined)).toBe(null); | ||
| expect(await render([[[false]], undefined])).toBe(null); |
There was a problem hiding this comment.
This just checks a complex component structure of no-render values? Pretty neat that this can just be data.
|
Change TEXT_NODE_TYPE to Element.TEXT_TYPE? Sure!
…On Tue, Aug 1, 2017 at 7:27 PM Dan Abramov ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js
<#10345 (comment)>:
> @@ -455,6 +455,22 @@ describe('ReactDOMServerIntegration', () => {
expect(parent.childNodes[1].tagName).toBe('DIV');
expect(parent.childNodes[2].tagName).toBe('DIV');
});
+
+ itRenders('emptyish values', async render => {
+ let e = await render(0);
+ expect(e.nodeType).toBe(3);
Want to send follow up? We actually already have this as a constant in the
same file. But literal is used in more than one test.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#10345 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAkEOJzWwXZNdNovJN0tZKJT1enMDUMeks5sT7RNgaJpZM4Oqas_>
.
|
|
I mean changing existing |
|
Ah. I don't mind doing that. Consider it done. |
|
There's another follow up here if you're interested. This test file is getting huge. We need to think about how to split it into a separate file for each |
|
|
||
| // TODO: This one is broken because client renders a node | ||
| // but server returns empty HTML. | ||
| // expect(await render('')).toBe(null); |
There was a problem hiding this comment.
In some test I've instead tested that parent.textContent === '' since that's what really matters. That there's no significant text content. More resilient to algorithm changes.
You could do:
expect((await render(<div>{''}</div>)).textContent).toBe('');
It used to have false positives for cases where we legitimately don't render anything.
Per #10339 (comment).