Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding new SSR tests for context, refs, and reviving markup. #9257

Conversation

aickin
Copy link
Contributor

@aickin aickin commented Mar 25, 2017

This is a latest chapter in a series of PRs adding server-side rendering unit tests (following on #9055, #9089, #9106, and #9221).

This PR adds SSR tests for context, refs, and markup mismatch warnings when reviving.

If we end up implementing the new SSR as a Fiber renderer, then the context tests are probably not needed, as I'm guessing that Fiber handles context by itself and is well tested. If we write SSR from scratch, though, those tests will be useful.

Also, while I found all these tests useful when I wrote my custom server renderer, I totally understand if you find them repetitive, especially the ones that repeat tests for both class components and stateless components. Just let me know what you think!

Copy link
Collaborator

@sophiebits sophiebits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks great, especially thanks for the lifecycle timing and mismatches-on-whitespace ones.

it('can distinguish an empty component from an empty text component', () =>
expectMarkupMismatch(<div><EmptyComponent /></div>, <div>{''}</div>));

it('should reconnect if component trees differ but resulting markup is the same', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this different from, ex: the ES6 vs Pure test above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! I could argue that it's a little different, in that it's comparing two different React.Component class implementations against each other, which wasn't done above, but yeah, it's probably duplicative and has a very low (maybe zero?) likelihood of catching something not caught by those tests. I can take it out in the next PR.

@sophiebits sophiebits merged commit c51411c into facebook:master Mar 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants