Add unit tests for props to attribute mapping in SSR. #9106
Conversation
… which will soon be merged into master.
…eworked wording of test names, corrected use of canUseDom, and simplified tests against tagName. Thanks for the help, @spicyj!
|
@spicyj, could you help me find a reviewer for this? I have probably four more PRs of unit tests behind it, so I'd love to see if we could find a way to move this one along. And as always, I'm open to suggestions of how to make the PRs more digestible. Thanks! |
|
This is just great! Would look forward to have improved tests for SSR in the code base! |
| @@ -61,14 +61,18 @@ function renderIntoDom(reactElement, domElement, errorCount = 0) { | |||
| ); | |||
| } | |||
|
|
|||
| async function renderIntoString(reactElement, errorCount = 0) { | |||
| return await expectErrors( | |||
| () => new Promise(resolve => resolve(ReactDOMServer.renderToString(reactElement))), | |||
There was a problem hiding this comment.
Was there an issue with Promise.resolve?
There was a problem hiding this comment.
Yeah, it behaves differently if renderToString throws an error. With Promise.resolve(), the arrow function throws, whereas with new Promise, the promise just rejects. There are some tests that I haven't yet submitted that depend on the promises rejecting on error rather than throwing up the call stack.
|
|
||
| // this seems like somewhat odd behavior, as it isn't how <a html> works | ||
| // in HTML, but it's existing behavior. | ||
| itRenders('string prop with true value', async render => { |
There was a problem hiding this comment.
(This test is probably unnecessary since it's seen the same by React as href: true and I'm sure we'll be careful about compat if we ever change JSX.)
There was a problem hiding this comment.
Totally fair point. I'll remove it.
|
|
||
| // this does not seem like correct behavior, since hidden="" in HTML indicates | ||
| // that the boolean property is present. however, it is how the current code | ||
| // behaves, so the test is included here. |
|
This has been approved and passed CI (and then failed after the move to prettier, but I just fixed that, too). @spicyj, would it be possible to merge? Thanks! |
This PR is a continuation of my port of SSR unit tests into master. There are two parts to this PR:
As always, any feedback is welcome. Thanks!